* [PATCH 1/6] dt-bindings: pinctrl: aspeed: Document AST2600 pinmux
From: Andrew Jeffery @ 2019-07-11 4:19 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190711041942.23202-1-andrew@aj.id.au>
The AST260 differs from the 2400 and 2500 in that it supports multiple
groups for a subset of functions.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
.../pinctrl/aspeed,ast2600-pinctrl.yaml | 128 ++++++++++++++++++
1 file changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
new file mode 100644
index 000000000000..dd31f8e62433
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/aspeed,ast2600-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2600 Pin Controller
+
+maintainers:
+ - Andrew Jeffery <andrew@aj.id.au>
+
+description: |+
+ The pin controller node should be the child of a syscon node with the
+ required property:
+
+ - compatible: Should be one of the following:
+ "aspeed,ast2600-scu", "syscon", "simple-mfd"
+
+ Refer to the the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.txt
+
+properties:
+ compatible:
+ const: aspeed,ast2600-pinctrl
+
+patternProperties:
+ '^.*$':
+ if:
+ type: object
+ then:
+ patternProperties:
+ "^function$":
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/string"
+ - enum: [ "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",
+ "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
+ "ADC8", "ADC9", "BMCINT", "ESPI", "ESPIALT", "FSI1", "FSI2",
+ "FWSPIABR", "FWSPID", "FWSPIWP", "GPIT0", "GPIT1", "GPIT2",
+ "GPIT3", "GPIT4", "GPIT5", "GPIT6", "GPIT7", "GPIU0", "GPIU1",
+ "GPIU2", "GPIU3", "GPIU4", "GPIU5", "GPIU6", "GPIU7", "I2C1",
+ "I2C10", "I2C11", "I2C12", "I2C13", "I2C14", "I2C15", "I2C16",
+ "I2C2", "I2C3", "I2C4", "I2C5", "I2C6", "I2C7", "I2C8", "I2C9",
+ "I3C3", "I3C4", "I3C5", "I3C6", "JTAGM", "LHPD", "LHSIRQ", "LPC",
+ "LPCHC", "LPCPD", "LPCPME", "LPCSMI", "LSIRQ", "MACLINK1",
+ "MACLINK2", "MACLINK3", "MACLINK4", "MDIO1", "MDIO2", "MDIO3",
+ "MDIO4", "NCTS1", "NCTS2", "NCTS3", "NCTS4", "NDCD1", "NDCD2",
+ "NDCD3", "NDCD4", "NDSR1", "NDSR2", "NDSR3", "NDSR4", "NDTR1",
+ "NDTR2", "NDTR3", "NDTR4", "NRI1", "NRI2", "NRI3", "NRI4",
+ "NRTS1", "NRTS2", "NRTS3", "NRTS4", "OSCCLK", "PEWAKE", "PWM0",
+ "PWM1", "PWM10", "PWM11", "PWM12", "PWM13", "PWM14", "PWM15",
+ "PWM2", "PWM3", "PWM4", "PWM5", "PWM6", "PWM7", "PWM8", "PWM9",
+ "RGMII1", "RGMII2", "RGMII3", "RGMII4", "RMII1", "RMII2",
+ "RMII3", "RMII4", "RXD1", "RXD2", "RXD3", "RXD4", "SALT1",
+ "SALT10", "SALT11", "SALT12", "SALT13", "SALT14", "SALT15",
+ "SALT16", "SALT2", "SALT3", "SALT4", "SALT5", "SALT6", "SALT7",
+ "SALT8", "SALT9", "SD1", "SD2", "SD3", "SD3DAT4", "SD3DAT5",
+ "SD3DAT6", "SD3DAT7", "SGPM1", "SGPS1", "SIOONCTRL", "SIOPBI",
+ "SIOPBO", "SIOPWREQ", "SIOPWRGD", "SIOS3", "SIOS5", "SIOSCI",
+ "SPI1", "SPI1ABR", "SPI1CS1", "SPI1WP", "SPI2", "SPI2CS1",
+ "SPI2CS2", "TACH0", "TACH1", "TACH10", "TACH11", "TACH12",
+ "TACH13", "TACH14", "TACH15", "TACH2", "TACH3", "TACH4", "TACH5",
+ "TACH6", "TACH7", "TACH8", "TACH9", "THRU0", "THRU1", "THRU2",
+ "THRU3", "TXD1", "TXD2", "TXD3", "TXD4", "UART10", "UART11",
+ "UART12", "UART13", "UART6", "UART7", "UART8", "UART9", "VB",
+ "VGAHS", "VGAVS", "WDTRST1", "WDTRST2", "WDTRST3", "WDTRST4", ]
+ "^groups$":
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/string"
+ - enum: [ "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",
+ "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
+ "ADC8", "ADC9", "BMCINT", "ESPI", "ESPIALT", "FSI1", "FSI2",
+ "FWSPIABR", "FWSPID", "FWQSPID", "FWSPIWP", "GPIT0", "GPIT1",
+ "GPIT2", "GPIT3", "GPIT4", "GPIT5", "GPIT6", "GPIT7", "GPIU0",
+ "GPIU1", "GPIU2", "GPIU3", "GPIU4", "GPIU5", "GPIU6", "GPIU7",
+ "HVI3C3", "HVI3C4", "I2C1", "I2C10", "I2C11", "I2C12", "I2C13",
+ "I2C14", "I2C15", "I2C16", "I2C2", "I2C3", "I2C4", "I2C5",
+ "I2C6", "I2C7", "I2C8", "I2C9", "I3C3", "I3C4", "I3C5", "I3C6",
+ "JTAGM", "LHPD", "LHSIRQ", "LPC", "LPCHC", "LPCPD", "LPCPME",
+ "LPCSMI", "LSIRQ", "MACLINK1", "MACLINK2", "MACLINK3",
+ "MACLINK4", "MDIO1", "MDIO2", "MDIO3", "MDIO4", "NCTS1", "NCTS2",
+ "NCTS3", "NCTS4", "NDCD1", "NDCD2", "NDCD3", "NDCD4", "NDSR1",
+ "NDSR2", "NDSR3", "NDSR4", "NDTR1", "NDTR2", "NDTR3", "NDTR4",
+ "NRI1", "NRI2", "NRI3", "NRI4", "NRTS1", "NRTS2", "NRTS3",
+ "NRTS4", "OSCCLK", "PEWAKE", "PWM0", "PWM1", "PWM10G0",
+ "PWM10G1", "PWM11G0", "PWM11G1", "PWM12G0", "PWM12G1", "PWM13G0",
+ "PWM13G1", "PWM14G0", "PWM14G1", "PWM15G0", "PWM15G1", "PWM2",
+ "PWM3", "PWM4", "PWM5", "PWM6", "PWM7", "PWM8G0", "PWM8G1",
+ "PWM9G0", "PWM9G1", "QSPI1", "QSPI2", "RGMII1", "RGMII2",
+ "RGMII3", "RGMII4", "RMII1", "RMII2", "RMII3", "RMII4", "RXD1",
+ "RXD2", "RXD3", "RXD4", "SALT1", "SALT10G0", "SALT10G1",
+ "SALT11G0", "SALT11G1", "SALT12G0", "SALT12G1", "SALT13G0",
+ "SALT13G1", "SALT14G0", "SALT14G1", "SALT15G0", "SALT15G1",
+ "SALT16G0", "SALT16G1", "SALT2", "SALT3", "SALT4", "SALT5",
+ "SALT6", "SALT7", "SALT8", "SALT9G0", "SALT9G1", "SD1", "SD2",
+ "SD3", "SD3DAT4", "SD3DAT5", "SD3DAT6", "SD3DAT7", "SGPM1",
+ "SGPS1", "SIOONCTRL", "SIOPBI", "SIOPBO", "SIOPWREQ", "SIOPWRGD",
+ "SIOS3", "SIOS5", "SIOSCI", "SPI1", "SPI1ABR", "SPI1CS1",
+ "SPI1WP", "SPI2", "SPI2CS1", "SPI2CS2", "TACH0", "TACH1",
+ "TACH10", "TACH11", "TACH12", "TACH13", "TACH14", "TACH15",
+ "TACH2", "TACH3", "TACH4", "TACH5", "TACH6", "TACH7", "TACH8",
+ "TACH9", "THRU0", "THRU1", "THRU2", "THRU3", "TXD1", "TXD2",
+ "TXD3", "TXD4", "UART10", "UART11", "UART12G0", "UART12G1",
+ "UART13G0", "UART13G1", "UART6", "UART7", "UART8", "UART9", "VB",
+ "VGAHS", "VGAVS", "WDTRST1", "WDTRST2", "WDTRST3", "WDTRST4", ]
+
+required:
+ - compatible
+
+examples:
+ - |
+ syscon: scu at 1e6e2000 {
+ compatible = "aspeed,ast2600-scu", "syscon", "simple-mfd";
+ reg = <0x1e6e2000 0xf6c>;
+
+ pinctrl: pinctrl {
+ compatible = "aspeed,g6-pinctrl";
+
+ pinctrl_pwm10g1_default: pwm10g1_default {
+ function = "PWM10";
+ groups = "PWM10G1";
+ };
+
+ pinctrl_gpioh0_unbiased_default: gpioh0 {
+ pins = "A18";
+ bias-disable;
+ };
+ };
+ };
--
2.20.1
^ permalink raw reply related
* [PATCH 0/6] pinctrl: aspeed: Add AST2600 pinmux support
From: Andrew Jeffery @ 2019-07-11 4:19 UTC (permalink / raw)
To: linux-aspeed
Hello!
This series adds pinmux support for the AST2600. Some more rework was required
on top of the previous cleanup series, but this rework was focussed on
supporting features of the AST2600 pinmux rather than fixing issues with the
existing infrastructure for the ASPEED drivers. Due to the dependences it's
based on top of pinctrl/devel, so should avoid any more SPDX issues.
ASPEED have been testing the patches on hardware, so even for an initial pass
there's some confidence in the implementation.
Please review!
Andrew
Andrew Jeffery (6):
dt-bindings: pinctrl: aspeed: Document AST2600 pinmux
pinctrl: aspeed: Rename pin declaration macros
pinctrl: aspeed: Add PIN_DECL_3() helper
pinctrl: aspeed: Add multiple pin group support for functions
pinctrl: aspeed: Add SIG_DESC_CLEAR() helper
pinctrl: aspeed: Add AST2600 pinmux support
.../pinctrl/aspeed,ast2600-pinctrl.yaml | 128 +
drivers/pinctrl/aspeed/Kconfig | 8 +
drivers/pinctrl/aspeed/Makefile | 1 +
drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 1821 +++++++------
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 1865 +++++++------
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 2395 +++++++++++++++++
drivers/pinctrl/aspeed/pinmux-aspeed.h | 227 +-
7 files changed, 4643 insertions(+), 1802 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
create mode 100644 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
--
2.20.1
^ permalink raw reply
* [PATCH 2/3 v2] ARM: dts: aspeed: Add SGPIO driver
From: Andrew Jeffery @ 2019-07-11 1:46 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1562772382-6555-1-git-send-email-hongweiz@ami.com>
On Thu, 11 Jul 2019, at 00:56, Hongwei Zhang wrote:
> Add SGPIO driver support for Aspeed AST2500 SoC.
>
> Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> ---
> drivers/gpio/sgpio-aspeed.c | 450 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 450 insertions(+)
> create mode 100644 drivers/gpio/sgpio-aspeed.c
>
> diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> new file mode 100644
> index 0000000..0743d22
> --- /dev/null
> +++ b/drivers/gpio/sgpio-aspeed.c
> @@ -0,0 +1,450 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 American Megatrends International LLC.
> + *
> + * Author: Karthikeyan Mani <karthikeyanm@amiindia.co.in>
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/aspeed.h>
> +#include <linux/hashtable.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +
> +#define NR_SGPIO 80
> +
> +struct aspeed_sgpio {
> + struct gpio_chip chip;
> + spinlock_t lock;
> + void __iomem *base;
> + int irq;
> +};
> +
> +struct aspeed_sgpio_bank {
> + uint16_t val_regs;
> + uint16_t rdata_reg;
> + uint16_t irq_regs;
> + const char names[4][3];
> +};
> +
> +/*
> + * Note: The "value" register returns the input value sampled on the
> + * line even when the GPIO is configured as an output. Since
> + * that input goes through synchronizers, writing, then reading
> + * back may not return the written value right away.
> + *
> + * The "rdata" register returns the content of the write latch
> + * and thus can be used to read back what was last written
> + * reliably.
> + */
> +
> +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> + {
> + .val_regs = 0x0000,
> + .rdata_reg = 0x0070,
> + .irq_regs = 0x0004,
> + .names = { "A", "B", "C", "D" },
> + },
> + {
> + .val_regs = 0x001C,
> + .rdata_reg = 0x0074,
> + .irq_regs = 0x0020,
> + .names = { "E", "F", "G", "H" },
> + },
> + {
> + .val_regs = 0x0038,
> + .rdata_reg = 0x0078,
> + .irq_regs = 0x003C,
> + .names = { "I", "J" },
> + },
> +};
> +
> +enum aspeed_sgpio_reg {
> + reg_val,
> + reg_rdata,
> + reg_irq_enable,
> + reg_irq_type0,
> + reg_irq_type1,
> + reg_irq_type2,
> + reg_irq_status,
> +};
> +
> +#define GPIO_VAL_VALUE 0x00
> +#define GPIO_VAL_DIR 0x04
> +#define GPIO_IRQ_ENABLE 0x00
> +#define GPIO_IRQ_TYPE0 0x04
> +#define GPIO_IRQ_TYPE1 0x08
> +#define GPIO_IRQ_TYPE2 0x0C
> +#define GPIO_IRQ_STATUS 0x10
> +
> +/* This will be resolved at compile time */
> +static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
> + const struct aspeed_sgpio_bank *bank,
> + const enum aspeed_sgpio_reg reg)
> +{
> + switch (reg) {
> + case reg_val:
> + return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
> + case reg_rdata:
> + return gpio->base + bank->rdata_reg;
> + case reg_irq_enable:
> + return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
> + case reg_irq_type0:
> + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
> + case reg_irq_type1:
> + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
> + case reg_irq_type2:
> + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
> + case reg_irq_status:
> + return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
> + default:
> + /* acturally if code runs to here, it's an error case */
> + WARN_ON(reg);
> + return gpio->base;
> + }
> +}
> +
> +#define GPIO_BANK(x) ((x) >> 5)
> +#define GPIO_OFFSET(x) ((x) & 0x1f)
> +#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
> +
> +static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
> +{
> + unsigned int bank = GPIO_BANK(offset);
> +
> + WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
> + return &aspeed_sgpio_banks[bank];
> +}
> +
> +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> +
> + return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
> +}
> +
> +static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> offset, int val)
> +{
> + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> + unsigned long flags;
> + void __iomem *addr;
> + u32 reg = 0;
> +
> + spin_lock_irqsave(&gpio->lock, flags);
> +
> + addr = bank_reg(gpio, bank, reg_val);
> +
> + if (val)
> + reg |= GPIO_BIT(offset);
> + else
> + reg &= ~GPIO_BIT(offset);
> +
> + iowrite32(reg, addr);
> + spin_unlock_irqrestore(&gpio->lock, flags);
> +}
> +
> +static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int
> offset)
> +{
> + /* By default all SGPIO Pins are input */
> + return 0;
> +}
> +
> +static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int
> offset, int val)
> +{
> + return 0;
> +}
> +
> +static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned
> int offset)
> +{
> + /* By default all SGPIO Pins are input */
> + return 1;
> +
> +}
Please see my follow-up reply on v1 that helps clarify what we should do
with the dir_in()/dir_out()/get_direction() implementations. The
implementation here will confuse everything in the stack above it.
> +
> +static inline int irqd_to_aspeed_sgpio_data(struct irq_data *d,
> + struct aspeed_sgpio **gpio,
> + const struct aspeed_sgpio_bank **bank,
> + u32 *bit, int *offset)
> +{
> + struct aspeed_sgpio *internal;
> +
> + *offset = irqd_to_hwirq(d);
> +
> + internal = irq_data_get_irq_chip_data(d);
> +
> + *gpio = internal;
> + *bank = to_bank(*offset);
> + *bit = GPIO_BIT(*offset);
> +
> + return 0;
> +}
> +
> +static void aspeed_sgpio_irq_ack(struct irq_data *d)
> +{
> + const struct aspeed_sgpio_bank *bank;
> + struct aspeed_sgpio *gpio;
> + unsigned long flags;
> + void __iomem *status_addr;
> + int rc, offset;
> + u32 bit;
> +
> + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> + if (rc)
> + return;
> +
> + status_addr = bank_reg(gpio, bank, reg_irq_status);
> +
> + spin_lock_irqsave(&gpio->lock, flags);
> +
> + iowrite32(bit, status_addr);
> +
> + spin_unlock_irqrestore(&gpio->lock, flags);
> +}
> +
> +static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set)
> +{
> + const struct aspeed_sgpio_bank *bank;
> + struct aspeed_sgpio *gpio;
> + unsigned long flags;
> + u32 reg, bit;
> + void __iomem *addr;
> + int rc, offset;
> +
> + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> + if (rc)
> + return;
> +
> + addr = bank_reg(gpio, bank, reg_irq_enable);
> +
> + spin_lock_irqsave(&gpio->lock, flags);
> +
> + reg = ioread32(addr);
> + if (set)
> + reg |= bit;
> + else
> + reg &= ~bit;
> +
> + iowrite32(reg, addr);
> +
> + spin_unlock_irqrestore(&gpio->lock, flags);
> +}
> +
> +static void aspeed_sgpio_irq_mask(struct irq_data *d)
> +{
> + aspeed_sgpio_irq_set_mask(d, false);
> +}
> +
> +static void aspeed_sgpio_irq_unmask(struct irq_data *d)
> +{
> + aspeed_sgpio_irq_set_mask(d, true);
> +}
> +
> +static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int type)
> +{
> + u32 type0 = 0;
> + u32 type1 = 0;
> + u32 type2 = 0;
> + u32 bit, reg;
> + const struct aspeed_sgpio_bank *bank;
> + irq_flow_handler_t handler;
> + struct aspeed_sgpio *gpio;
> + unsigned long flags;
> + void __iomem *addr;
> + int rc, offset;
> +
> + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> + if (rc)
> + return -EINVAL;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_EDGE_BOTH:
> + type2 |= bit;
> + /* fall through */
> + case IRQ_TYPE_EDGE_RISING:
> + type0 |= bit;
> + /* fall through */
> + case IRQ_TYPE_EDGE_FALLING:
> + handler = handle_edge_irq;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + type0 |= bit;
> + /* fall through */
> + case IRQ_TYPE_LEVEL_LOW:
> + type1 |= bit;
> + handler = handle_level_irq;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + spin_lock_irqsave(&gpio->lock, flags);
> +
> + addr = bank_reg(gpio, bank, reg_irq_type0);
> + reg = ioread32(addr);
> + reg = (reg & ~bit) | type0;
> + iowrite32(reg, addr);
> +
> + addr = bank_reg(gpio, bank, reg_irq_type1);
> + reg = ioread32(addr);
> + reg = (reg & ~bit) | type1;
> + iowrite32(reg, addr);
> +
> + addr = bank_reg(gpio, bank, reg_irq_type2);
> + reg = ioread32(addr);
> + reg = (reg & ~bit) | type2;
> + iowrite32(reg, addr);
> +
> + spin_unlock_irqrestore(&gpio->lock, flags);
> +
> + irq_set_handler_locked(d, handler);
> +
> + return 0;
> +}
> +
> +static void aspeed_sgpio_irq_handler(struct irq_desc *desc)
> +{
> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> + struct irq_chip *ic = irq_desc_get_chip(desc);
> + struct aspeed_sgpio *data = gpiochip_get_data(gc);
> + unsigned int i, p, girq;
> + unsigned long reg;
> +
> + chained_irq_enter(ic, desc);
> +
> + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> + const struct aspeed_sgpio_bank *bank = &aspeed_sgpio_banks[i];
> +
> + reg = ioread32(bank_reg(data, bank, reg_irq_status));
> +
> + for_each_set_bit(p, ®, 32) {
> + girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
> + generic_handle_irq(girq);
> + }
> +
> + }
> +
> + chained_irq_exit(ic, desc);
> +}
> +
> +static struct irq_chip aspeed_sgpio_irqchip = {
> + .name = "aspeed-sgpio",
> + .irq_ack = aspeed_sgpio_irq_ack,
> + .irq_mask = aspeed_sgpio_irq_mask,
> + .irq_unmask = aspeed_sgpio_irq_unmask,
> + .irq_set_type = aspeed_sgpio_set_type,
> +};
> +
> +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> + struct platform_device *pdev)
> +{
> + int rc, i;
> + const struct aspeed_sgpio_bank *bank;
> +
> + rc = platform_get_irq(pdev, 0);
> + if (rc < 0)
> + return rc;
> +
> + gpio->irq = rc;
> +
> + /* Disable IRQ and clear Interrupt status registers for all SPGIO
> Pins. */
> + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> + bank = &aspeed_sgpio_banks[i];
> + /* disable irq enable bits */
> + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_enable));
> + /* clear status bits */
> + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_status));
> + }
> +
> + rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_sgpio_irqchip,
> + 0, handle_bad_irq, IRQ_TYPE_NONE);
> + if (rc) {
> + dev_info(&pdev->dev, "Could not add irqchip\n");
> + return rc;
> + }
> +
> + gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_sgpio_irqchip,
> + gpio->irq, aspeed_sgpio_irq_handler);
> +
> + /* set IRQ settings and Enable Interrupt */
> + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> + bank = &aspeed_sgpio_banks[i];
> + /* set falling or level-low irq */
> + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type0));
> + /* trigger type is edge */
> + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1));
> + /* dual edge trigger mode. */
> + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2));
> + /* enable irq */
> + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable));
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_sgpio_request(struct gpio_chip *chip, unsigned int
> offset)
> +{
> + return (offset < NR_SGPIO);
> +}
I don't think this request() implementation is helpful, especially as it stands
in the face of needing to pull the number of GPIOs to serialise from the
devicetree.
request() is an optional callback, lets just drop it.
> +
> +static const struct of_device_id aspeed_sgpio_of_table[] = {
> + { .compatible = "aspeed,ast2400-sgpio" },
> + { .compatible = "aspeed,ast2500-sgpio" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_sgpio_of_table);
> +
> +static int __init aspeed_sgpio_probe(struct platform_device *pdev)
> +{
> + struct aspeed_sgpio *gpio;
> + struct resource *res;
> + int rc;
> +
> + gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
> + if (!gpio)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + gpio->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(gpio->base))
> + return PTR_ERR(gpio->base);
> +
> + spin_lock_init(&gpio->lock);
> +
> + gpio->chip.parent = &pdev->dev;
> + gpio->chip.ngpio = NR_SGPIO;
> + gpio->chip.direction_input = aspeed_sgpio_dir_in;
> + gpio->chip.direction_output = aspeed_sgpio_dir_out;
> + gpio->chip.get_direction = aspeed_sgpio_get_direction;
> + gpio->chip.request = aspeed_sgpio_request;
> + gpio->chip.free = NULL;
> + gpio->chip.get = aspeed_sgpio_get;
> + gpio->chip.set = aspeed_sgpio_set;
> + gpio->chip.set_config = NULL;
> + gpio->chip.label = dev_name(&pdev->dev);
> + gpio->chip.base = -1;
> +
> + rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> + if (rc < 0)
> + return rc;
> +
> + return aspeed_sgpio_setup_irqs(gpio, pdev);
> +}
> +
> +static struct platform_driver aspeed_sgpio_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = aspeed_sgpio_of_table,
> + },
> +};
> +
> +module_platform_driver_probe(aspeed_sgpio_driver, aspeed_sgpio_probe);
> +MODULE_DESCRIPTION("Aspeed Serial GPIO Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
>
^ permalink raw reply
* [PATCH 2/3 v1] ARM: dts: aspeed: Add SGPIO driver
From: Andrew Jeffery @ 2019-07-11 1:38 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <14D3C8298A3B0F42A1EB31EE961CFF82AA8F4335@atlms2.us.megatrends.com>
On Thu, 11 Jul 2019, at 00:24, Hongwei Zhang wrote:
> Hello Andrew,
>
> Thanks for your review and comments, please find our inline response at
> below.
Out of interest, who is Karthik? Are they the one developing the code? If so
the patch should have their authorship/Signed-off-by. It's fine if you send it,
git will attribute the code to the right people just fine.
> I will email updated driver code separately, because Outlook breaks
> source code's tabs.
No worries. Can you use e.g. a gmail account instead? Might make things
easier for you.
>
> There is one place need your more input for clarification, which is
> about DATA_READ/DATA_VALUE registers,
> please see it at below.
>
> Best Regards,
> -- Hongwei
>
> > From: Andrew Jeffery <andrew@aj.id.au>
> > Sent: Wednesday, July 3, 2019 8:06 PM
> > To: Hongwei Zhang; Bartosz Golaszewski; Joel Stanley; Linus Walleij
> > Cc: linux-gpio at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-aspeed at lists.ozlabs.org;
> > linux-kernel at vger.kernel.org
> > Subject: Re: [PATCH 2/3 linux,dev-5.1 v1] ARM: dts: aspeed: Add SGPIO driver
> >
> > Hello Hongwei,
> >
> > As this is patch is sent to the upstream lists (linux-gpio@ etc) please drop the OpenBMC-specific
> > "linux,dev-5.1" from the subject.
> >
>
> Got it but to be more specific, for the situation of mixed recipients,
> should I send out separate emails with
> different subject line format in the future?
It boils down to:
* If the patches are for the upstream kernel, follow upstream's process.
* If the patches are for the OpenBMC kernel, follow the OpenBMC kernel
development process.
Those processes are independent, though vastly similar. Make sure to
read through the relevant documentation for each. A big part of OpenBMC's
kernel development process is "send your patches upstream" :)
>
> > Also, it looks like you may have manually added the series revision (v1).
> > For the record you can make `git format-patch` do this for you with the `-v`option (e.g. if you really want
> > it here, `-v 1`).
> >
> > On Thu, 4 Jul 2019, at 07:09, Hongwei Zhang wrote:
> > > Add SGPIO driver support for Aspeed AST2500 SoC.
> > >
> > > Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> > > ---
> > > drivers/gpio/sgpio-aspeed.c | 470
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 470 insertions(+)
> > > create mode 100644 drivers/gpio/sgpio-aspeed.c
> > >
> > > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > > new file mode 100644 index 0000000..108ed13
> > > --- /dev/null
> > > +++ b/drivers/gpio/sgpio-aspeed.c
> > > @@ -0,0 +1,470 @@
> > > +/*
> > > + * Copyright 2019 American Megatrends International LLC.
> > > + *
> > > + * 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.
> >
> > You should use the SPDX license identifier here rather than the GPL blurb, and it should be the first line
> > of the file. Keep your copyright line in place though:
> >
> OK
>
> > // SPDX-License-Identifier: GPL-2.0-or-later // Copyright 2019 American Megatrends International LLC.
> >
> > > + */
> > > +
> > > +#include <linux/gpio/driver.h>
> > > +#include <linux/gpio/aspeed.h>
> > > +#include <linux/hashtable.h>
> > > +#include <linux/init.h>
> > > +#include <linux/io.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pinctrl/consumer.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/string.h>
> > > +
> > > +#define NR_SGPIO 80
> > > +
> > > +struct aspeed_sgpio {
> > > + struct gpio_chip chip;
> > > + spinlock_t lock;
> > > + void __iomem *base;
> > > + int irq;
> > > +};
> > > +
> > > +struct aspeed_sgpio_bank {
> > > + uint16_t val_regs;
> > > + uint16_t rdata_reg;
> > > + uint16_t irq_regs;
> > > + const char names[4][3];
> > > +};
> > > +
> > > +/*
> > > + * Note: The "value" register returns the input value sampled on the
> > > + * line even when the GPIO is configured as an output. Since
> > > + * that input goes through synchronizers, writing, then reading
> > > + * back may not return the written value right away.
> > > + *
> > > + * The "rdata" register returns the content of the write latch
> > > + * and thus can be used to read back what was last written
> > > + * reliably.
> > > + */
> > > +
> > > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > > + {
> > > + .val_regs = 0x0000,
> > > + .rdata_reg = 0x0070,
> > > + .irq_regs = 0x0004,
> > > + .names = { "A", "B", "C", "D" },
> > > + },
> > > + {
> > > + .val_regs = 0x001C,
> > > + .rdata_reg = 0x0074,
> > > + .irq_regs = 0x0020,
> > > + .names = { "E", "F", "G", "H" },
> > > + },
> > > + {
> > > + .val_regs = 0x0038,
> > > + .rdata_reg = 0x0078,
> > > + .irq_regs = 0x003C,
> > > + .names = { "I", "J" },
> > > + },
> > > +};
> > > +
> > > +enum aspeed_sgpio_reg {
> > > + reg_val,
> > > + reg_rdata,
> > > + reg_irq_enable,
> > > + reg_irq_type0,
> > > + reg_irq_type1,
> > > + reg_irq_type2,
> > > + reg_irq_status,
> > > +};
> > > +
> > > +#define GPIO_VAL_VALUE 0x00
> > > +#define GPIO_VAL_DIR 0x04
> > > +#define GPIO_IRQ_ENABLE 0x00
> > > +#define GPIO_IRQ_TYPE0 0x04
> > > +#define GPIO_IRQ_TYPE1 0x08
> > > +#define GPIO_IRQ_TYPE2 0x0C
> > > +#define GPIO_IRQ_STATUS 0x10
> > > +
> > > +/* This will be resolved at compile time */ static inline void
> > > +__iomem *bank_reg(struct aspeed_sgpio *gpio,
> > > + const struct aspeed_sgpio_bank *bank,
> > > + const enum aspeed_sgpio_reg reg) {
> > > + switch (reg) {
> > > + case reg_val:
> > > + return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
> > > + case reg_rdata:
> > > + return gpio->base + bank->rdata_reg;
> > > + case reg_irq_enable:
> > > + return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
> > > + case reg_irq_type0:
> > > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
> > > + case reg_irq_type1:
> > > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
> > > + case reg_irq_type2:
> > > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
> > > + case reg_irq_status:
> > > + return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
> > > + }
> > > + BUG_ON(1);
> >
> > This isn't appropriate - we shouldn't take down the kernel on a faulty peripheral access. Please change
> > this to WARN().
> >
> - Cannot change it to WARN(), it throws build error,
> warning: control reaches end of non-void
> function [-Wreturn-type], so I add WARN_ON() in the
> switch's 'default: ' case statement, and
> followed with a return(gpio->base) to mute the
> [-Wreturn-type] compiling warning.
Hmm, maybe I fired that thought off before thinking about it enough.
The parallel GPIO driver pretty much does the same as the BUG_ON(1)
above. The alternative you propose is data corruption, which isn't
what we want either (data corruption with off-chip side-effects no less).
On reflection I think it's best to leave it as BUG() (which is the simple
version of your BUG_ON(1)). Returning NULL just leads to more of a
headache when we dereference the pointer somewhere else in the
code.
>
> > > +}
> > > +
> > > +#define GPIO_BANK(x) ((x) >> 5)
> > > +#define GPIO_OFFSET(x) ((x) & 0x1f)
> > > +#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
> > > +
> > > +static const struct aspeed_sgpio_bank *to_bank(unsigned int offset) {
> > > + unsigned int bank = GPIO_BANK(offset);
> > > +
> > > + WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
> > > + return &aspeed_sgpio_banks[bank];
> > > +}
> > > +
> > > +static inline bool have_gpio(struct aspeed_sgpio *gpio, unsigned int
> > > offset)
> > > +{
> > > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > > + unsigned int group = GPIO_OFFSET(offset) / 8;
> > > +
> > > + return bank->names[group][0] != '\0';
> >
> > Lets just drop have_gpio() altogether, it's a contiguous set of 80 GPIOs.
> > At best this should just be:
> >
> > static inline bool have_gpio(struct aspeed_sgpio *gpio, unsigned int offset) {
> > return offset < NR_SGPIO;
> > }
> >
> > But lets just assume that we've properly configured the gpio subsystem for the controller and remove it
> > completely.
> >
>
> Karthik - [Addressed] Removed have_gpio()
Did you see my comments yesterday on the bindings patch? Addressing
them will affect how you go about configuring the GPIO subsystem for the
controller. Essentially we need to know from the devicetree how many
GPIOs we need to serialise (and also the bus frequency).
>
> > > +}
> > > +
> > > +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int
> > > +offset) {
> > > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > > +
> > > + return !!(ioread32(bank_reg(gpio, bank, reg_val)) &
> > > +GPIO_BIT(offset)); }
> > > +
> > > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > > offset,
> > > + int val)
> >
> > No need to split this out from aspeed_sgpio_set() below. Separating the implementation was necessary
> > in the parallel GPIO driver for reasons that aren't relevant here.
> >
>
> Karthik - [Addressed] Merged into single API itself.
>
> > > +{
> > > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > > + void __iomem *addr;
> > > + u32 reg;
> > > +
> > > + addr = bank_reg(gpio, bank, reg_val);
> > > +
> > > + if (val)
> > > + reg |= GPIO_BIT(offset);
> > > + else
> > > + reg &= ~GPIO_BIT(offset);
> > > +
> > > + iowrite32(reg, addr);
> > > +}
> > > +
> > > +static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset,
> > > + int val)
> > > +{
> > > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&gpio->lock, flags);
> > > +
> > > + __aspeed_sgpio_set(gc, offset, val);
> > > +
> > > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > > +
> > > +static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int
> > > offset)
> > > +{
> > > + /* By default all SGPIO Pins are input */
> >
> > Right, but with your implementation below you can never mark them as output.
> >
>
> Karthik - [Addressed] Just return success if user space try to set dir
> as input or output.
But you still need to track it, yes? This is how we know whether to
read reg_val or reg_rdata.
>
> > > + return 0;
> > > +}
> > > +
> > > +static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned
> > > int offset)
> > > +{
> > > + /* By default all SGPIO Pins are input */
> > > + return 1;
> >
> > As above. Given my understanding of SGPIO, I think you should be implementing both dir_in() and
> > dir_out(), and capturing which state userspace "wants" the GPIO to be in, and directing reads/writes to
> > the DATA_READ/DATA_VALUE registers as appropriate. There's no state we need to modify in the
> > hardware, but that doesn't mean we shouldn't capture the intent of userspace at all.
> >
>
> Karthik - Just return success if user space try to set dir as input or
> output.
> But I don't understand the point directing
> reads/writes to the DATA_READ/DATA_VALUE Registers.
> If userspace configured GPIO as input, Then get_gpio()
> should return reg_rdata value?
The value in reg_rdata is the last value written to reg_val. So if the
GPIO is configured as input, we want to read reg_val, as this will
contain our input value. If the GPIO is configured as output, then
we want to read reg_rdata to make sure we send the "right" value
back to userspace independent of the state of the SGPIO bus.
Hope that helps.
Andrew
> If userspace configured GPIO as output, Then
> get_gpio() should return reg_val value?
> Please clarify.
>
> > > +
> > > +}
> > > +
> > > +static inline int irqd_to_aspeed_sgpio_data(struct irq_data *d,
> > > + struct aspeed_sgpio **gpio,
> > > + const struct aspeed_sgpio_bank **bank,
> > > + u32 *bit, int *offset)
> > > +{
> > > + struct aspeed_sgpio *internal;
> > > +
> > > + *offset = irqd_to_hwirq(d);
> > > +
> > > + internal = irq_data_get_irq_chip_data(d);
> > > +
> > > + *gpio = internal;
> > > + *bank = to_bank(*offset);
> > > + *bit = GPIO_BIT(*offset);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void aspeed_sgpio_irq_ack(struct irq_data *d) {
> > > + const struct aspeed_sgpio_bank *bank;
> > > + struct aspeed_sgpio *gpio;
> > > + unsigned long flags;
> > > + void __iomem *status_addr;
> > > + int rc, offset;
> > > + u32 bit;
> > > +
> > > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > > + if (rc)
> > > + return;
> > > +
> > > + status_addr = bank_reg(gpio, bank, reg_irq_status);
> > > +
> > > + spin_lock_irqsave(&gpio->lock, flags);
> > > +
> > > + iowrite32(bit, status_addr);
> > > +
> > > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > > +
> > > +static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set) {
> > > + const struct aspeed_sgpio_bank *bank;
> > > + struct aspeed_sgpio *gpio;
> > > + unsigned long flags;
> > > + u32 reg, bit;
> > > + void __iomem *addr;
> > > + int rc, offset;
> > > +
> > > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > > + if (rc)
> > > + return;
> > > +
> > > + addr = bank_reg(gpio, bank, reg_irq_enable);
> > > +
> > > + spin_lock_irqsave(&gpio->lock, flags);
> > > +
> > > + reg = ioread32(addr);
> > > + if (set)
> > > + reg |= bit;
> > > + else
> > > + reg &= ~bit;
> > > +
> > > + iowrite32(reg, addr);
> > > +
> > > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > > +
> > > +static void aspeed_sgpio_irq_mask(struct irq_data *d) {
> > > + aspeed_sgpio_irq_set_mask(d, false); }
> > > +
> > > +static void aspeed_sgpio_irq_unmask(struct irq_data *d) {
> > > + aspeed_sgpio_irq_set_mask(d, true);
> > > +}
> > > +
> > > +static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int
> > > +type) {
> > > + u32 type0 = 0;
> > > + u32 type1 = 0;
> > > + u32 type2 = 0;
> > > + u32 bit, reg;
> > > + const struct aspeed_sgpio_bank *bank;
> > > + irq_flow_handler_t handler;
> > > + struct aspeed_sgpio *gpio;
> > > + unsigned long flags;
> > > + void __iomem *addr;
> > > + int rc, offset;
> > > +
> > > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > > + if (rc)
> > > + return -EINVAL;
> > > +
> > > + switch (type & IRQ_TYPE_SENSE_MASK) {
> > > + case IRQ_TYPE_EDGE_BOTH:
> > > + type2 |= bit;
> > > + /* fall through */
> > > + case IRQ_TYPE_EDGE_RISING:
> > > + type0 |= bit;
> > > + /* fall through */
> > > + case IRQ_TYPE_EDGE_FALLING:
> > > + handler = handle_edge_irq;
> > > + break;
> > > + case IRQ_TYPE_LEVEL_HIGH:
> > > + type0 |= bit;
> > > + /* fall through */
> > > + case IRQ_TYPE_LEVEL_LOW:
> > > + type1 |= bit;
> > > + handler = handle_level_irq;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + spin_lock_irqsave(&gpio->lock, flags);
> > > +
> > > + addr = bank_reg(gpio, bank, reg_irq_type0);
> > > + reg = ioread32(addr);
> > > + reg = (reg & ~bit) | type0;
> > > + iowrite32(reg, addr);
> > > +
> > > + addr = bank_reg(gpio, bank, reg_irq_type1);
> > > + reg = ioread32(addr);
> > > + reg = (reg & ~bit) | type1;
> > > + iowrite32(reg, addr);
> > > +
> > > + addr = bank_reg(gpio, bank, reg_irq_type2);
> > > + reg = ioread32(addr);
> > > + reg = (reg & ~bit) | type2;
> > > + iowrite32(reg, addr);
> > > +
> > > + spin_unlock_irqrestore(&gpio->lock, flags);
> > > +
> > > + irq_set_handler_locked(d, handler);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void aspeed_sgpio_irq_handler(struct irq_desc *desc) {
> > > + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> > > + struct irq_chip *ic = irq_desc_get_chip(desc);
> > > + struct aspeed_sgpio *data = gpiochip_get_data(gc);
> > > + unsigned int i, p, girq;
> > > + unsigned long reg;
> > > +
> > > + chained_irq_enter(ic, desc);
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > > + const struct aspeed_sgpio_bank *bank = &aspeed_sgpio_banks[i];
> > > +
> > > + reg = ioread32(bank_reg(data, bank, reg_irq_status));
> > > +
> > > + for_each_set_bit(p, ®, 32) {
> > > + girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
> > > + generic_handle_irq(girq);
> > > + }
> > > +
> > > + }
> > > +
> > > + chained_irq_exit(ic, desc);
> > > +}
> > > +
> > > +static struct irq_chip aspeed_sgpio_irqchip = {
> > > + .name = "aspeed-sgpio",
> > > + .irq_ack = aspeed_sgpio_irq_ack,
> > > + .irq_mask = aspeed_sgpio_irq_mask,
> > > + .irq_unmask = aspeed_sgpio_irq_unmask,
> > > + .irq_set_type = aspeed_sgpio_set_type,
> > > +};
> > > +
> > > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> > > + struct platform_device *pdev)
> > > +{
> > > + int rc, i;
> > > + const struct aspeed_sgpio_bank *bank;
> > > +
> > > + rc = platform_get_irq(pdev, 0);
> > > + if (rc < 0)
> > > + return rc;
> > > +
> > > + gpio->irq = rc;
> > > +
> > > + /* Disable IRQ and clear Interrupt status registers for all SPGIO
> > > Pins. */
> > > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > > + bank = &aspeed_sgpio_banks[i];
> > > + /* disable irq enable bits */
> > > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_enable));
> > > + /* clear status bits */
> > > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_status));
> > > + }
> > > +
> > > + rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_sgpio_irqchip,
> > > + 0, handle_bad_irq, IRQ_TYPE_NONE);
> > > + if (rc) {
> > > + dev_info(&pdev->dev, "Could not add irqchip\n");
> > > + return rc;
> > > + }
> > > +
> > > + gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_sgpio_irqchip,
> > > + gpio->irq, aspeed_sgpio_irq_handler);
> > > +
> > > + /* set IRQ settings and Enable Interrupt */
> > > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > > + bank = &aspeed_sgpio_banks[i];
> > > + /* set falling or level-low irq */
> > > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type0));
> > > + /* trigger type is edge */
> > > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1));
> > > + /* dual edge trigger mode. */
> > > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2));
> > > + /* enable irq */
> > > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable));
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int aspeed_sgpio_request(struct gpio_chip *chip, unsigned int
> > > offset)
> > > +{
> > > + if (!have_gpio(gpiochip_get_data(chip), offset))
> > > + return -ENODEV;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id aspeed_sgpio_of_table[] = {
> > > + { .compatible = "aspeed,ast2400-sgpio", .data = NULL, },
> > > + { .compatible = "aspeed,ast2500-sgpio", .data = NULL,},
> >
> > You can drop the assignment to data.
> >
>
> Karthik - [Addressed] dropped data parameter
>
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, aspeed_sgpio_of_table);
> > > +
> > > +static int __init aspeed_sgpio_probe(struct platform_device *pdev) {
> > > + const struct of_device_id *gpio_id;
> > > + struct aspeed_sgpio *gpio;
> > > + struct resource *res;
> > > + int rc, i;
> > > +
> > > + gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
> > > + if (!gpio)
> > > + return -ENOMEM;
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + gpio->base = devm_ioremap_resource(&pdev->dev, res);
> > > + if (IS_ERR(gpio->base))
> > > + return PTR_ERR(gpio->base);
> > > +
> > > + spin_lock_init(&gpio->lock);
> > > +
> > > + gpio_id = of_match_node(aspeed_sgpio_of_table, pdev->dev.of_node);
> > > + if (!gpio_id)
> > > + return -EINVAL;
> >
> > gpio_id isn't used, so you can drop the of_match_node() above.
> >
>
> Karthik - [Addressed] dropped gpio_id
>
> > > +
> > > + gpio->chip.parent = &pdev->dev;
> > > + gpio->chip.ngpio = NR_SGPIO;
> > > + gpio->chip.direction_input = aspeed_sgpio_dir_in;
> > > + gpio->chip.direction_output = NULL;
> >
> > We can do outputs too - we shouldn't be omitting the direction_output callback, see the discussion
> > above about dir_in()/dir_out()/get_direction()
> >
>
> Karthik - [Addressed] Added output API also.
>
> > Andrew
> >
> > > + gpio->chip.get_direction = aspeed_sgpio_get_direction;
> > > + gpio->chip.request = aspeed_sgpio_request;
> > > + gpio->chip.free = NULL;
> > > + gpio->chip.get = aspeed_sgpio_get;
> > > + gpio->chip.set = aspeed_sgpio_set;
> > > + gpio->chip.set_config = NULL;
> > > + gpio->chip.label = dev_name(&pdev->dev);
> > > + gpio->chip.base = -1;
> > > +
> > > + rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> > > + if (rc < 0)
> > > + return rc;
> > > +
> > > + return aspeed_sgpio_setup_irqs(gpio, pdev); }
> > > +
> > > +static struct platform_driver aspeed_sgpio_driver = {
> > > + .driver = {
> > > + .name = KBUILD_MODNAME,
> > > + .of_match_table = aspeed_sgpio_of_table,
> > > + },
> > > +};
> > > +
> > > +module_platform_driver_probe(aspeed_sgpio_driver,
> > > +aspeed_sgpio_probe); MODULE_DESCRIPTION("Aspeed Serial GPIO Driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.7.4
> > >
> > >
>
^ permalink raw reply
* [PATCH 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Andrew Jeffery @ 2019-07-11 0:56 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAL_JsqKrYcUbn_02z9GQO6U4rz7k=p3kB7_G0tKmv25MMZZNQg@mail.gmail.com>
On Thu, 11 Jul 2019, at 01:20, Rob Herring wrote:
> On Wed, Jul 10, 2019 at 8:16 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> > The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the
> > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> > data bus if only a single slot is enabled.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > .../bindings/mmc/sdhci-of-aspeed.yaml | 91 +++++++++++++++++++
>
> aspeed,sdhci.yaml
Ack. Previously I had separate documents for different compatibles, wasn't sure
how to tackle name one document covering multiple compatibles.
>
> > 1 file changed, 91 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
> > new file mode 100644
> > index 000000000000..e98a2ac4d46d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: GPL-2.0-or-later
>
> The preferred license is (GPL-2.0 OR BSD-2-Clause) if that is okay with you.
I should poke people internally again, I had asked, just haven't got a clear
answer. What was the justification for the preference (just so I can pass that
on)?
>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/sdhci-of-aspeed.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED SD/SDIO/eMMC Controller
> > +
> > +maintainers:
> > + - Andrew Jeffery <andrew@aj.id.au>
> > + - Ryan Chen <ryanchen.aspeed@gmail.com>
> > +
> > +description: |+
> > + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO
> > + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if
> > + only a single slot is enabled.
> > +
> > + The two slots are supported by a common configuration area. As the SDHCIs for
> > + the slots are dependent on the common configuration area, they are described
> > + as child nodes.
> > +
> > +properties:
> > + compatible:
> > + enum: [ aspeed,ast2400-sdc, aspeed,ast2500-sdc ]
> > + reg:
> > + description: Common configuration registers
>
> This should have a 'maxItems: 1'. Same for the child reg.
Ack.
>
> > + ranges: true
> > + clocks:
> > + maxItems: 1
> > + description: The SD/SDIO controller clock gate
> > + sdhci:
>
> This needs to be a pattern (under patternProperties) as these have
> unit-addresses.
Ah, I didn't think about that.
>
> > + type: object
> > + properties:
> > + compatible:
> > + allOf:
> > + - enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ]
> > + - const: sdhci
>
> This condition can never be true. What you need is s/allOf/items/.
> However, 'sdhci' is not really a useful compatible because every
> implementation has quirks, so I'd drop it.
Yeah, I was tossing up whether to include "sdhci". I'll drop it as you
suggest.
Pity the shorthand doesn't work how I expected. Might explain
some of the behaviour I was seeing with the bindings make targets
though.
>
> > + reg:
> > + description: The SDHCI registers
> > + clocks:
> > + maxItems: 1
> > + description: The SD bus clock
> > + slot:
> > + allOf:
> > + - $ref: /schemas/types.yaml#/definitions/uint32
> > + - enum: [0, 1]
>
> Is this really needed? Offset 0x100 is slot 0 and offset 0x200 is slot
> 1. Does that ever change?
It doesn't in the SoCs at hand, but the downstream impact (driver
implementation) is that you need to derive the slot index from the
address and I was trying to avoid that. The slot index influences
some bit index calculations.
>
> > + interrupts:
> > + maxItems: 1
> > + description: The SD interrupt shared between both slots
> > + required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - slot
> > + - interrupts
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - ranges
> > + - clocks
>
> #address-cells and #size-cells are required too.
Ack.
>
> You should also add 'additionalProperties: false' here so other random
> properties can't be present.
Ah yes. Will fix.
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/aspeed-clock.h>
> > + sdc at 1e740000 {
> > + compatible = "aspeed,ast2500-sdc";
> > + reg = <0x1e740000 0x100>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
>
> It's preferred to limit the range here and then the child addresses
> are 0x100 and 0x200.
Okay. Was just trying to dodge mental arithmetic where I could, but
if that's the preference then I'll fix it up.
Thanks for the review!
Andrew
>
> > + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
> > +
> > + sdhci0: sdhci at 1e740100 {
> > + compatible = "aspeed,ast2500-sdhci", "sdhci";
> > + reg = <0x1e740100 0x100>;
> > + slot = <0>;
> > + interrupts = <26>;
> > + sdhci,auto-cmd12;
> > + clocks = <&syscon ASPEED_CLK_SDIO>;
> > + };
> > +
> > + sdhci1: sdhci at 1e740200 {
> > + compatible = "aspeed,ast2500-sdhci", "sdhci";
> > + reg = <0x1e740200 0x100>;
> > + slot = <1>;
> > + interrupts = <26>;
> > + sdhci,auto-cmd12;
> > + clocks = <&syscon ASPEED_CLK_SDIO>;
> > + };
> > + };
> > --
> > 2.20.1
> >
>
^ permalink raw reply
* [PATCH 00/12] treewide: Fix GENMASK misuses
From: Joe Perches @ 2019-07-10 16:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <b9c3b83c9be50286062ae8cefd5d38e2baa0fb22.camel@perches.com>
On Wed, 2019-07-10 at 08:45 -0700, Joe Perches wrote:
> On Wed, 2019-07-10 at 10:43 +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote:
> > > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> > > > These GENMASK uses are inverted argument order and the
> > > > actual masks produced are incorrect. Fix them.
> > > >
> > > > Add checkpatch tests to help avoid more misuses too.
> > > >
> > > > Joe Perches (12):
> > > > checkpatch: Add GENMASK tests
> > >
> > > IMHO this doesn't make a lot of sense as a checkpatch test - just throw
> > > in a BUILD_BUG_ON()?
>
> I tried that.
>
> It'd can't be done as it's used in declarations
> and included in asm files and it uses the UL()
> macro.
>
> I also tried just making it do the right thing
> whatever the argument order.
I forgot.
I also made all those arguments when it was
introduced in 2013.
https://lore.kernel.org/patchwork/patch/414248/
> Oh well.
yeah.
^ permalink raw reply
* [PATCH 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Rob Herring @ 2019-07-10 15:49 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141611.21159-2-andrew@aj.id.au>
On Wed, Jul 10, 2019 at 8:16 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the
> SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> data bus if only a single slot is enabled.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> .../bindings/mmc/sdhci-of-aspeed.yaml | 91 +++++++++++++++++++
aspeed,sdhci.yaml
> 1 file changed, 91 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
> new file mode 100644
> index 000000000000..e98a2ac4d46d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
The preferred license is (GPL-2.0 OR BSD-2-Clause) if that is okay with you.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/sdhci-of-aspeed.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED SD/SDIO/eMMC Controller
> +
> +maintainers:
> + - Andrew Jeffery <andrew@aj.id.au>
> + - Ryan Chen <ryanchen.aspeed@gmail.com>
> +
> +description: |+
> + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO
> + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if
> + only a single slot is enabled.
> +
> + The two slots are supported by a common configuration area. As the SDHCIs for
> + the slots are dependent on the common configuration area, they are described
> + as child nodes.
> +
> +properties:
> + compatible:
> + enum: [ aspeed,ast2400-sdc, aspeed,ast2500-sdc ]
> + reg:
> + description: Common configuration registers
This should have a 'maxItems: 1'. Same for the child reg.
> + ranges: true
> + clocks:
> + maxItems: 1
> + description: The SD/SDIO controller clock gate
> + sdhci:
This needs to be a pattern (under patternProperties) as these have
unit-addresses.
> + type: object
> + properties:
> + compatible:
> + allOf:
> + - enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ]
> + - const: sdhci
This condition can never be true. What you need is s/allOf/items/.
However, 'sdhci' is not really a useful compatible because every
implementation has quirks, so I'd drop it.
> + reg:
> + description: The SDHCI registers
> + clocks:
> + maxItems: 1
> + description: The SD bus clock
> + slot:
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - enum: [0, 1]
Is this really needed? Offset 0x100 is slot 0 and offset 0x200 is slot
1. Does that ever change?
> + interrupts:
> + maxItems: 1
> + description: The SD interrupt shared between both slots
> + required:
> + - compatible
> + - reg
> + - clocks
> + - slot
> + - interrupts
> +
> +required:
> + - compatible
> + - reg
> + - ranges
> + - clocks
#address-cells and #size-cells are required too.
You should also add 'additionalProperties: false' here so other random
properties can't be present.
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/aspeed-clock.h>
> + sdc at 1e740000 {
> + compatible = "aspeed,ast2500-sdc";
> + reg = <0x1e740000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
It's preferred to limit the range here and then the child addresses
are 0x100 and 0x200.
> + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
> +
> + sdhci0: sdhci at 1e740100 {
> + compatible = "aspeed,ast2500-sdhci", "sdhci";
> + reg = <0x1e740100 0x100>;
> + slot = <0>;
> + interrupts = <26>;
> + sdhci,auto-cmd12;
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + };
> +
> + sdhci1: sdhci at 1e740200 {
> + compatible = "aspeed,ast2500-sdhci", "sdhci";
> + reg = <0x1e740200 0x100>;
> + slot = <1>;
> + interrupts = <26>;
> + sdhci,auto-cmd12;
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + };
> + };
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH 00/12] treewide: Fix GENMASK misuses
From: Joe Perches @ 2019-07-10 15:45 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710094337.wf2lftxzfjq2etro@shell.armlinux.org.uk>
On Wed, 2019-07-10 at 10:43 +0100, Russell King - ARM Linux admin wrote:
> On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote:
> > On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> > > These GENMASK uses are inverted argument order and the
> > > actual masks produced are incorrect. Fix them.
> > >
> > > Add checkpatch tests to help avoid more misuses too.
> > >
> > > Joe Perches (12):
> > > checkpatch: Add GENMASK tests
> >
> > IMHO this doesn't make a lot of sense as a checkpatch test - just throw
> > in a BUILD_BUG_ON()?
I tried that.
It'd can't be done as it's used in declarations
and included in asm files and it uses the UL()
macro.
I also tried just making it do the right thing
whatever the argument order.
Oh well.
> My personal take on this is that GENMASK() is really not useful, it's
> just pure obfuscation and leads to exactly these kinds of mistakes.
>
> Yes, I fully understand the argument that you can just specify the
> start and end bits, and it _in theory_ makes the code more readable.
>
> However, the problem is when writing code. GENMASK(a, b). Is a the
> starting bit or ending bit? Is b the number of bits? It's confusing
> and causes mistakes resulting in incorrect code. A BUILD_BUG_ON()
> can catch some of the cases, but not all of them.
It's a horrid little macro and I agree with Russell.
I also think if it existed at all it should have been
GENMASK(low, high) not GENMASK(high, low).
I
^ permalink raw reply
* [PATCH 2/3 v2] ARM: dts: aspeed: Add SGPIO driver
From: Hongwei Zhang @ 2019-07-10 15:26 UTC (permalink / raw)
To: linux-aspeed
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
---
drivers/gpio/sgpio-aspeed.c | 450 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 450 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 0000000..0743d22
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani <karthikeyanm@amiindia.co.in>
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/gpio/aspeed.h>
+#include <linux/hashtable.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+
+#define NR_SGPIO 80
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ spinlock_t lock;
+ void __iomem *base;
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_t val_regs;
+ uint16_t rdata_reg;
+ uint16_t irq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value sampled on the
+ * line even when the GPIO is configured as an output. Since
+ * that input goes through synchronizers, writing, then reading
+ * back may not return the written value right away.
+ *
+ * The "rdata" register returns the content of the write latch
+ * and thus can be used to read back what was last written
+ * reliably.
+ */
+
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x0000,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_VAL_DIR 0x04
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+ const struct aspeed_sgpio_bank *bank,
+ const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ WARN_ON(reg);
+ return gpio->base;
+ }
+}
+
+#define GPIO_BANK(x) ((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return &aspeed_sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+
+ return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 reg = 0;
+
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ addr = bank_reg(gpio, bank, reg_val);
+
+ if (val)
+ reg |= GPIO_BIT(offset);
+ else
+ reg &= ~GPIO_BIT(offset);
+
+ iowrite32(reg, addr);
+ spin_unlock_irqrestore(&gpio->lock, flags);
+}
+
+static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+ /* By default all SGPIO Pins are input */
+ return 0;
+}
+
+static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ return 0;
+}
+
+static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+ /* By default all SGPIO Pins are input */
+ return 1;
+
+}
+
+static inline int irqd_to_aspeed_sgpio_data(struct irq_data *d,
+ struct aspeed_sgpio **gpio,
+ const struct aspeed_sgpio_bank **bank,
+ u32 *bit, int *offset)
+{
+ struct aspeed_sgpio *internal;
+
+ *offset = irqd_to_hwirq(d);
+
+ internal = irq_data_get_irq_chip_data(d);
+
+ *gpio = internal;
+ *bank = to_bank(*offset);
+ *bit = GPIO_BIT(*offset);
+
+ return 0;
+}
+
+static void aspeed_sgpio_irq_ack(struct irq_data *d)
+{
+ const struct aspeed_sgpio_bank *bank;
+ struct aspeed_sgpio *gpio;
+ unsigned long flags;
+ void __iomem *status_addr;
+ int rc, offset;
+ u32 bit;
+
+ rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
+ if (rc)
+ return;
+
+ status_addr = bank_reg(gpio, bank, reg_irq_status);
+
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ iowrite32(bit, status_addr);
+
+ spin_unlock_irqrestore(&gpio->lock, flags);
+}
+
+static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set)
+{
+ const struct aspeed_sgpio_bank *bank;
+ struct aspeed_sgpio *gpio;
+ unsigned long flags;
+ u32 reg, bit;
+ void __iomem *addr;
+ int rc, offset;
+
+ rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
+ if (rc)
+ return;
+
+ addr = bank_reg(gpio, bank, reg_irq_enable);
+
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ reg = ioread32(addr);
+ if (set)
+ reg |= bit;
+ else
+ reg &= ~bit;
+
+ iowrite32(reg, addr);
+
+ spin_unlock_irqrestore(&gpio->lock, flags);
+}
+
+static void aspeed_sgpio_irq_mask(struct irq_data *d)
+{
+ aspeed_sgpio_irq_set_mask(d, false);
+}
+
+static void aspeed_sgpio_irq_unmask(struct irq_data *d)
+{
+ aspeed_sgpio_irq_set_mask(d, true);
+}
+
+static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int type)
+{
+ u32 type0 = 0;
+ u32 type1 = 0;
+ u32 type2 = 0;
+ u32 bit, reg;
+ const struct aspeed_sgpio_bank *bank;
+ irq_flow_handler_t handler;
+ struct aspeed_sgpio *gpio;
+ unsigned long flags;
+ void __iomem *addr;
+ int rc, offset;
+
+ rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
+ if (rc)
+ return -EINVAL;
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_EDGE_BOTH:
+ type2 |= bit;
+ /* fall through */
+ case IRQ_TYPE_EDGE_RISING:
+ type0 |= bit;
+ /* fall through */
+ case IRQ_TYPE_EDGE_FALLING:
+ handler = handle_edge_irq;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ type0 |= bit;
+ /* fall through */
+ case IRQ_TYPE_LEVEL_LOW:
+ type1 |= bit;
+ handler = handle_level_irq;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ addr = bank_reg(gpio, bank, reg_irq_type0);
+ reg = ioread32(addr);
+ reg = (reg & ~bit) | type0;
+ iowrite32(reg, addr);
+
+ addr = bank_reg(gpio, bank, reg_irq_type1);
+ reg = ioread32(addr);
+ reg = (reg & ~bit) | type1;
+ iowrite32(reg, addr);
+
+ addr = bank_reg(gpio, bank, reg_irq_type2);
+ reg = ioread32(addr);
+ reg = (reg & ~bit) | type2;
+ iowrite32(reg, addr);
+
+ spin_unlock_irqrestore(&gpio->lock, flags);
+
+ irq_set_handler_locked(d, handler);
+
+ return 0;
+}
+
+static void aspeed_sgpio_irq_handler(struct irq_desc *desc)
+{
+ struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+ struct irq_chip *ic = irq_desc_get_chip(desc);
+ struct aspeed_sgpio *data = gpiochip_get_data(gc);
+ unsigned int i, p, girq;
+ unsigned long reg;
+
+ chained_irq_enter(ic, desc);
+
+ for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
+ const struct aspeed_sgpio_bank *bank = &aspeed_sgpio_banks[i];
+
+ reg = ioread32(bank_reg(data, bank, reg_irq_status));
+
+ for_each_set_bit(p, ®, 32) {
+ girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
+ generic_handle_irq(girq);
+ }
+
+ }
+
+ chained_irq_exit(ic, desc);
+}
+
+static struct irq_chip aspeed_sgpio_irqchip = {
+ .name = "aspeed-sgpio",
+ .irq_ack = aspeed_sgpio_irq_ack,
+ .irq_mask = aspeed_sgpio_irq_mask,
+ .irq_unmask = aspeed_sgpio_irq_unmask,
+ .irq_set_type = aspeed_sgpio_set_type,
+};
+
+static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
+ struct platform_device *pdev)
+{
+ int rc, i;
+ const struct aspeed_sgpio_bank *bank;
+
+ rc = platform_get_irq(pdev, 0);
+ if (rc < 0)
+ return rc;
+
+ gpio->irq = rc;
+
+ /* Disable IRQ and clear Interrupt status registers for all SPGIO Pins. */
+ for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
+ bank = &aspeed_sgpio_banks[i];
+ /* disable irq enable bits */
+ iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_enable));
+ /* clear status bits */
+ iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_status));
+ }
+
+ rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_sgpio_irqchip,
+ 0, handle_bad_irq, IRQ_TYPE_NONE);
+ if (rc) {
+ dev_info(&pdev->dev, "Could not add irqchip\n");
+ return rc;
+ }
+
+ gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_sgpio_irqchip,
+ gpio->irq, aspeed_sgpio_irq_handler);
+
+ /* set IRQ settings and Enable Interrupt */
+ for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
+ bank = &aspeed_sgpio_banks[i];
+ /* set falling or level-low irq */
+ iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type0));
+ /* trigger type is edge */
+ iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1));
+ /* dual edge trigger mode. */
+ iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2));
+ /* enable irq */
+ iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable));
+ }
+
+ return 0;
+}
+
+static int aspeed_sgpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+ return (offset < NR_SGPIO);
+}
+
+static const struct of_device_id aspeed_sgpio_of_table[] = {
+ { .compatible = "aspeed,ast2400-sgpio" },
+ { .compatible = "aspeed,ast2500-sgpio" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, aspeed_sgpio_of_table);
+
+static int __init aspeed_sgpio_probe(struct platform_device *pdev)
+{
+ struct aspeed_sgpio *gpio;
+ struct resource *res;
+ int rc;
+
+ gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+ if (!gpio)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ gpio->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(gpio->base))
+ return PTR_ERR(gpio->base);
+
+ spin_lock_init(&gpio->lock);
+
+ gpio->chip.parent = &pdev->dev;
+ gpio->chip.ngpio = NR_SGPIO;
+ gpio->chip.direction_input = aspeed_sgpio_dir_in;
+ gpio->chip.direction_output = aspeed_sgpio_dir_out;
+ gpio->chip.get_direction = aspeed_sgpio_get_direction;
+ gpio->chip.request = aspeed_sgpio_request;
+ gpio->chip.free = NULL;
+ gpio->chip.get = aspeed_sgpio_get;
+ gpio->chip.set = aspeed_sgpio_set;
+ gpio->chip.set_config = NULL;
+ gpio->chip.label = dev_name(&pdev->dev);
+ gpio->chip.base = -1;
+
+ rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
+ if (rc < 0)
+ return rc;
+
+ return aspeed_sgpio_setup_irqs(gpio, pdev);
+}
+
+static struct platform_driver aspeed_sgpio_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = aspeed_sgpio_of_table,
+ },
+};
+
+module_platform_driver_probe(aspeed_sgpio_driver, aspeed_sgpio_probe);
+MODULE_DESCRIPTION("Aspeed Serial GPIO Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3 v1] ARM: dts: aspeed: Add SGPIO driver
From: Hongwei Zhang @ 2019-07-10 14:54 UTC (permalink / raw)
To: linux-aspeed
Hello Andrew,
Thanks for your review and comments, please find our inline response at below.
I will email updated driver code separately, because Outlook breaks source code's tabs.
There is one place need your more input for clarification, which is about DATA_READ/DATA_VALUE registers,
please see it at below.
Best Regards,
-- Hongwei
> From: Andrew Jeffery <andrew@aj.id.au>
> Sent: Wednesday, July 3, 2019 8:06 PM
> To: Hongwei Zhang; Bartosz Golaszewski; Joel Stanley; Linus Walleij
> Cc: linux-gpio at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-aspeed at lists.ozlabs.org;
> linux-kernel at vger.kernel.org
> Subject: Re: [PATCH 2/3 linux,dev-5.1 v1] ARM: dts: aspeed: Add SGPIO driver
>
> Hello Hongwei,
>
> As this is patch is sent to the upstream lists (linux-gpio@ etc) please drop the OpenBMC-specific
> "linux,dev-5.1" from the subject.
>
Got it but to be more specific, for the situation of mixed recipients, should I send out separate emails with
different subject line format in the future?
> Also, it looks like you may have manually added the series revision (v1).
> For the record you can make `git format-patch` do this for you with the `-v`option (e.g. if you really want
> it here, `-v 1`).
>
> On Thu, 4 Jul 2019, at 07:09, Hongwei Zhang wrote:
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> > ---
> > drivers/gpio/sgpio-aspeed.c | 470
> > ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 470 insertions(+)
> > create mode 100644 drivers/gpio/sgpio-aspeed.c
> >
> > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > new file mode 100644 index 0000000..108ed13
> > --- /dev/null
> > +++ b/drivers/gpio/sgpio-aspeed.c
> > @@ -0,0 +1,470 @@
> > +/*
> > + * Copyright 2019 American Megatrends International LLC.
> > + *
> > + * 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.
>
> You should use the SPDX license identifier here rather than the GPL blurb, and it should be the first line
> of the file. Keep your copyright line in place though:
>
OK
> // SPDX-License-Identifier: GPL-2.0-or-later // Copyright 2019 American Megatrends International LLC.
>
> > + */
> > +
> > +#include <linux/gpio/driver.h>
> > +#include <linux/gpio/aspeed.h>
> > +#include <linux/hashtable.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pinctrl/consumer.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/string.h>
> > +
> > +#define NR_SGPIO 80
> > +
> > +struct aspeed_sgpio {
> > + struct gpio_chip chip;
> > + spinlock_t lock;
> > + void __iomem *base;
> > + int irq;
> > +};
> > +
> > +struct aspeed_sgpio_bank {
> > + uint16_t val_regs;
> > + uint16_t rdata_reg;
> > + uint16_t irq_regs;
> > + const char names[4][3];
> > +};
> > +
> > +/*
> > + * Note: The "value" register returns the input value sampled on the
> > + * line even when the GPIO is configured as an output. Since
> > + * that input goes through synchronizers, writing, then reading
> > + * back may not return the written value right away.
> > + *
> > + * The "rdata" register returns the content of the write latch
> > + * and thus can be used to read back what was last written
> > + * reliably.
> > + */
> > +
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > + {
> > + .val_regs = 0x0000,
> > + .rdata_reg = 0x0070,
> > + .irq_regs = 0x0004,
> > + .names = { "A", "B", "C", "D" },
> > + },
> > + {
> > + .val_regs = 0x001C,
> > + .rdata_reg = 0x0074,
> > + .irq_regs = 0x0020,
> > + .names = { "E", "F", "G", "H" },
> > + },
> > + {
> > + .val_regs = 0x0038,
> > + .rdata_reg = 0x0078,
> > + .irq_regs = 0x003C,
> > + .names = { "I", "J" },
> > + },
> > +};
> > +
> > +enum aspeed_sgpio_reg {
> > + reg_val,
> > + reg_rdata,
> > + reg_irq_enable,
> > + reg_irq_type0,
> > + reg_irq_type1,
> > + reg_irq_type2,
> > + reg_irq_status,
> > +};
> > +
> > +#define GPIO_VAL_VALUE 0x00
> > +#define GPIO_VAL_DIR 0x04
> > +#define GPIO_IRQ_ENABLE 0x00
> > +#define GPIO_IRQ_TYPE0 0x04
> > +#define GPIO_IRQ_TYPE1 0x08
> > +#define GPIO_IRQ_TYPE2 0x0C
> > +#define GPIO_IRQ_STATUS 0x10
> > +
> > +/* This will be resolved at compile time */ static inline void
> > +__iomem *bank_reg(struct aspeed_sgpio *gpio,
> > + const struct aspeed_sgpio_bank *bank,
> > + const enum aspeed_sgpio_reg reg) {
> > + switch (reg) {
> > + case reg_val:
> > + return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
> > + case reg_rdata:
> > + return gpio->base + bank->rdata_reg;
> > + case reg_irq_enable:
> > + return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
> > + case reg_irq_type0:
> > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
> > + case reg_irq_type1:
> > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
> > + case reg_irq_type2:
> > + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
> > + case reg_irq_status:
> > + return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
> > + }
> > + BUG_ON(1);
>
> This isn't appropriate - we shouldn't take down the kernel on a faulty peripheral access. Please change
> this to WARN().
>
- Cannot change it to WARN(), it throws build error, warning: control reaches end of non-void
function [-Wreturn-type], so I add WARN_ON() in the switch's 'default: ' case statement, and
followed with a return(gpio->base) to mute the [-Wreturn-type] compiling warning.
> > +}
> > +
> > +#define GPIO_BANK(x) ((x) >> 5)
> > +#define GPIO_OFFSET(x) ((x) & 0x1f)
> > +#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
> > +
> > +static const struct aspeed_sgpio_bank *to_bank(unsigned int offset) {
> > + unsigned int bank = GPIO_BANK(offset);
> > +
> > + WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
> > + return &aspeed_sgpio_banks[bank];
> > +}
> > +
> > +static inline bool have_gpio(struct aspeed_sgpio *gpio, unsigned int
> > offset)
> > +{
> > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > + unsigned int group = GPIO_OFFSET(offset) / 8;
> > +
> > + return bank->names[group][0] != '\0';
>
> Lets just drop have_gpio() altogether, it's a contiguous set of 80 GPIOs.
> At best this should just be:
>
> static inline bool have_gpio(struct aspeed_sgpio *gpio, unsigned int offset) {
> return offset < NR_SGPIO;
> }
>
> But lets just assume that we've properly configured the gpio subsystem for the controller and remove it
> completely.
>
Karthik - [Addressed] Removed have_gpio()
> > +}
> > +
> > +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int
> > +offset) {
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > +
> > + return !!(ioread32(bank_reg(gpio, bank, reg_val)) &
> > +GPIO_BIT(offset)); }
> > +
> > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > offset,
> > + int val)
>
> No need to split this out from aspeed_sgpio_set() below. Separating the implementation was necessary
> in the parallel GPIO driver for reasons that aren't relevant here.
>
Karthik - [Addressed] Merged into single API itself.
> > +{
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > + void __iomem *addr;
> > + u32 reg;
> > +
> > + addr = bank_reg(gpio, bank, reg_val);
> > +
> > + if (val)
> > + reg |= GPIO_BIT(offset);
> > + else
> > + reg &= ~GPIO_BIT(offset);
> > +
> > + iowrite32(reg, addr);
> > +}
> > +
> > +static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset,
> > + int val)
> > +{
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&gpio->lock, flags);
> > +
> > + __aspeed_sgpio_set(gc, offset, val);
> > +
> > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > +
> > +static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int
> > offset)
> > +{
> > + /* By default all SGPIO Pins are input */
>
> Right, but with your implementation below you can never mark them as output.
>
Karthik - [Addressed] Just return success if user space try to set dir as input or output.
> > + return 0;
> > +}
> > +
> > +static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned
> > int offset)
> > +{
> > + /* By default all SGPIO Pins are input */
> > + return 1;
>
> As above. Given my understanding of SGPIO, I think you should be implementing both dir_in() and
> dir_out(), and capturing which state userspace "wants" the GPIO to be in, and directing reads/writes to
> the DATA_READ/DATA_VALUE registers as appropriate. There's no state we need to modify in the
> hardware, but that doesn't mean we shouldn't capture the intent of userspace at all.
>
Karthik - Just return success if user space try to set dir as input or output.
But I don't understand the point directing reads/writes to the DATA_READ/DATA_VALUE Registers.
If userspace configured GPIO as input, Then get_gpio() should return reg_rdata value?
If userspace configured GPIO as output, Then get_gpio() should return reg_val value?
Please clarify.
> > +
> > +}
> > +
> > +static inline int irqd_to_aspeed_sgpio_data(struct irq_data *d,
> > + struct aspeed_sgpio **gpio,
> > + const struct aspeed_sgpio_bank **bank,
> > + u32 *bit, int *offset)
> > +{
> > + struct aspeed_sgpio *internal;
> > +
> > + *offset = irqd_to_hwirq(d);
> > +
> > + internal = irq_data_get_irq_chip_data(d);
> > +
> > + *gpio = internal;
> > + *bank = to_bank(*offset);
> > + *bit = GPIO_BIT(*offset);
> > +
> > + return 0;
> > +}
> > +
> > +static void aspeed_sgpio_irq_ack(struct irq_data *d) {
> > + const struct aspeed_sgpio_bank *bank;
> > + struct aspeed_sgpio *gpio;
> > + unsigned long flags;
> > + void __iomem *status_addr;
> > + int rc, offset;
> > + u32 bit;
> > +
> > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > + if (rc)
> > + return;
> > +
> > + status_addr = bank_reg(gpio, bank, reg_irq_status);
> > +
> > + spin_lock_irqsave(&gpio->lock, flags);
> > +
> > + iowrite32(bit, status_addr);
> > +
> > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > +
> > +static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set) {
> > + const struct aspeed_sgpio_bank *bank;
> > + struct aspeed_sgpio *gpio;
> > + unsigned long flags;
> > + u32 reg, bit;
> > + void __iomem *addr;
> > + int rc, offset;
> > +
> > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > + if (rc)
> > + return;
> > +
> > + addr = bank_reg(gpio, bank, reg_irq_enable);
> > +
> > + spin_lock_irqsave(&gpio->lock, flags);
> > +
> > + reg = ioread32(addr);
> > + if (set)
> > + reg |= bit;
> > + else
> > + reg &= ~bit;
> > +
> > + iowrite32(reg, addr);
> > +
> > + spin_unlock_irqrestore(&gpio->lock, flags); }
> > +
> > +static void aspeed_sgpio_irq_mask(struct irq_data *d) {
> > + aspeed_sgpio_irq_set_mask(d, false); }
> > +
> > +static void aspeed_sgpio_irq_unmask(struct irq_data *d) {
> > + aspeed_sgpio_irq_set_mask(d, true);
> > +}
> > +
> > +static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int
> > +type) {
> > + u32 type0 = 0;
> > + u32 type1 = 0;
> > + u32 type2 = 0;
> > + u32 bit, reg;
> > + const struct aspeed_sgpio_bank *bank;
> > + irq_flow_handler_t handler;
> > + struct aspeed_sgpio *gpio;
> > + unsigned long flags;
> > + void __iomem *addr;
> > + int rc, offset;
> > +
> > + rc = irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
> > + if (rc)
> > + return -EINVAL;
> > +
> > + switch (type & IRQ_TYPE_SENSE_MASK) {
> > + case IRQ_TYPE_EDGE_BOTH:
> > + type2 |= bit;
> > + /* fall through */
> > + case IRQ_TYPE_EDGE_RISING:
> > + type0 |= bit;
> > + /* fall through */
> > + case IRQ_TYPE_EDGE_FALLING:
> > + handler = handle_edge_irq;
> > + break;
> > + case IRQ_TYPE_LEVEL_HIGH:
> > + type0 |= bit;
> > + /* fall through */
> > + case IRQ_TYPE_LEVEL_LOW:
> > + type1 |= bit;
> > + handler = handle_level_irq;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + spin_lock_irqsave(&gpio->lock, flags);
> > +
> > + addr = bank_reg(gpio, bank, reg_irq_type0);
> > + reg = ioread32(addr);
> > + reg = (reg & ~bit) | type0;
> > + iowrite32(reg, addr);
> > +
> > + addr = bank_reg(gpio, bank, reg_irq_type1);
> > + reg = ioread32(addr);
> > + reg = (reg & ~bit) | type1;
> > + iowrite32(reg, addr);
> > +
> > + addr = bank_reg(gpio, bank, reg_irq_type2);
> > + reg = ioread32(addr);
> > + reg = (reg & ~bit) | type2;
> > + iowrite32(reg, addr);
> > +
> > + spin_unlock_irqrestore(&gpio->lock, flags);
> > +
> > + irq_set_handler_locked(d, handler);
> > +
> > + return 0;
> > +}
> > +
> > +static void aspeed_sgpio_irq_handler(struct irq_desc *desc) {
> > + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> > + struct irq_chip *ic = irq_desc_get_chip(desc);
> > + struct aspeed_sgpio *data = gpiochip_get_data(gc);
> > + unsigned int i, p, girq;
> > + unsigned long reg;
> > +
> > + chained_irq_enter(ic, desc);
> > +
> > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > + const struct aspeed_sgpio_bank *bank = &aspeed_sgpio_banks[i];
> > +
> > + reg = ioread32(bank_reg(data, bank, reg_irq_status));
> > +
> > + for_each_set_bit(p, ®, 32) {
> > + girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
> > + generic_handle_irq(girq);
> > + }
> > +
> > + }
> > +
> > + chained_irq_exit(ic, desc);
> > +}
> > +
> > +static struct irq_chip aspeed_sgpio_irqchip = {
> > + .name = "aspeed-sgpio",
> > + .irq_ack = aspeed_sgpio_irq_ack,
> > + .irq_mask = aspeed_sgpio_irq_mask,
> > + .irq_unmask = aspeed_sgpio_irq_unmask,
> > + .irq_set_type = aspeed_sgpio_set_type,
> > +};
> > +
> > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> > + struct platform_device *pdev)
> > +{
> > + int rc, i;
> > + const struct aspeed_sgpio_bank *bank;
> > +
> > + rc = platform_get_irq(pdev, 0);
> > + if (rc < 0)
> > + return rc;
> > +
> > + gpio->irq = rc;
> > +
> > + /* Disable IRQ and clear Interrupt status registers for all SPGIO
> > Pins. */
> > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > + bank = &aspeed_sgpio_banks[i];
> > + /* disable irq enable bits */
> > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_enable));
> > + /* clear status bits */
> > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_status));
> > + }
> > +
> > + rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_sgpio_irqchip,
> > + 0, handle_bad_irq, IRQ_TYPE_NONE);
> > + if (rc) {
> > + dev_info(&pdev->dev, "Could not add irqchip\n");
> > + return rc;
> > + }
> > +
> > + gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_sgpio_irqchip,
> > + gpio->irq, aspeed_sgpio_irq_handler);
> > +
> > + /* set IRQ settings and Enable Interrupt */
> > + for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > + bank = &aspeed_sgpio_banks[i];
> > + /* set falling or level-low irq */
> > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type0));
> > + /* trigger type is edge */
> > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1));
> > + /* dual edge trigger mode. */
> > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2));
> > + /* enable irq */
> > + iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_sgpio_request(struct gpio_chip *chip, unsigned int
> > offset)
> > +{
> > + if (!have_gpio(gpiochip_get_data(chip), offset))
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id aspeed_sgpio_of_table[] = {
> > + { .compatible = "aspeed,ast2400-sgpio", .data = NULL, },
> > + { .compatible = "aspeed,ast2500-sgpio", .data = NULL,},
>
> You can drop the assignment to data.
>
Karthik - [Addressed] dropped data parameter
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, aspeed_sgpio_of_table);
> > +
> > +static int __init aspeed_sgpio_probe(struct platform_device *pdev) {
> > + const struct of_device_id *gpio_id;
> > + struct aspeed_sgpio *gpio;
> > + struct resource *res;
> > + int rc, i;
> > +
> > + gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
> > + if (!gpio)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + gpio->base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(gpio->base))
> > + return PTR_ERR(gpio->base);
> > +
> > + spin_lock_init(&gpio->lock);
> > +
> > + gpio_id = of_match_node(aspeed_sgpio_of_table, pdev->dev.of_node);
> > + if (!gpio_id)
> > + return -EINVAL;
>
> gpio_id isn't used, so you can drop the of_match_node() above.
>
Karthik - [Addressed] dropped gpio_id
> > +
> > + gpio->chip.parent = &pdev->dev;
> > + gpio->chip.ngpio = NR_SGPIO;
> > + gpio->chip.direction_input = aspeed_sgpio_dir_in;
> > + gpio->chip.direction_output = NULL;
>
> We can do outputs too - we shouldn't be omitting the direction_output callback, see the discussion
> above about dir_in()/dir_out()/get_direction()
>
Karthik - [Addressed] Added output API also.
> Andrew
>
> > + gpio->chip.get_direction = aspeed_sgpio_get_direction;
> > + gpio->chip.request = aspeed_sgpio_request;
> > + gpio->chip.free = NULL;
> > + gpio->chip.get = aspeed_sgpio_get;
> > + gpio->chip.set = aspeed_sgpio_set;
> > + gpio->chip.set_config = NULL;
> > + gpio->chip.label = dev_name(&pdev->dev);
> > + gpio->chip.base = -1;
> > +
> > + rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return aspeed_sgpio_setup_irqs(gpio, pdev); }
> > +
> > +static struct platform_driver aspeed_sgpio_driver = {
> > + .driver = {
> > + .name = KBUILD_MODNAME,
> > + .of_match_table = aspeed_sgpio_of_table,
> > + },
> > +};
> > +
> > +module_platform_driver_probe(aspeed_sgpio_driver,
> > +aspeed_sgpio_probe); MODULE_DESCRIPTION("Aspeed Serial GPIO Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.7.4
> >
> >
^ permalink raw reply
* [PATCH 2/2] mmc: Add support for the ASPEED SD controller
From: Andrew Jeffery @ 2019-07-10 14:16 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141611.21159-1-andrew@aj.id.au>
Add a minimal driver for ASPEED's SD controller, which exposes two
SDHCIs.
The ASPEED design implements a common register set for the SDHCIs, and
moves some of the standard configuration elements out to this common
area (e.g. 8-bit mode, and card detect configuration which is not
currently supported).
The SD controller has a dedicated hardware interrupt that is shared
between the slots. The common register set exposes information on which
slot triggered the interrupt; early revisions of the patch introduced an
irqchip for the register, but reality is it doesn't behave as an
irqchip, and the result fits awkwardly into the irqchip APIs. Instead
I've taken the simple approach of using the IRQ as a shared IRQ with
some minor performance impact for the second slot.
Ryan was the original author of the patch - I've taken his work and
massaged it to drop the irqchip support and rework the devicetree
integration. The driver has been smoke tested under qemu against a
minimal SD controller model and lightly tested on an ast2500-evb.
Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/mmc/host/Kconfig | 12 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-of-aspeed.c | 307 +++++++++++++++++++++++++++++
3 files changed, 320 insertions(+)
create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 931770f17087..2bb5e1264b3d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -154,6 +154,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 73578718f119..390ee162fe71 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -84,6 +84,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 000000000000..23fad19787db
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (C) 2019 ASPEED Technology Inc. */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/mmc/host.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#include "sdhci-pltfm.h"
+
+#define ASPEED_SDC_INFO 0x00
+#define ASPEED_SDC_S1MMC8 BIT(25)
+#define ASPEED_SDC_S0MMC8 BIT(24)
+
+struct aspeed_sdc {
+ struct clk *clk;
+
+ spinlock_t lock;
+ void __iomem *regs;
+};
+
+struct aspeed_sdhci {
+ struct aspeed_sdc *parent;
+ u32 width_mask;
+};
+
+static void aspeed_sdc_bus_width(struct aspeed_sdc *sdc,
+ struct aspeed_sdhci *sdhci, bool bus8)
+{
+ u32 info;
+
+ /* Set/clear 8 bit mode */
+ spin_lock(&sdc->lock);
+ info = readl(sdc->regs + ASPEED_SDC_INFO);
+ if (bus8)
+ info |= sdhci->width_mask;
+ else
+ info &= ~sdhci->width_mask;
+ writel(info, sdc->regs + ASPEED_SDC_INFO);
+ spin_unlock(&sdc->lock);
+}
+
+static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ unsigned long timeout;
+ int div;
+ u16 clk;
+
+ 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 aspeed_sdhci_set_bus_width(struct sdhci_host *host, int width)
+{
+ struct sdhci_pltfm_host *pltfm_priv;
+ struct aspeed_sdhci *aspeed_sdhci;
+ struct aspeed_sdc *aspeed_sdc;
+ u8 ctrl;
+
+ pltfm_priv = sdhci_priv(host);
+ aspeed_sdhci = sdhci_pltfm_priv(pltfm_priv);
+ aspeed_sdc = aspeed_sdhci->parent;
+
+ /* Set/clear 8-bit mode */
+ aspeed_sdc_bus_width(aspeed_sdc, aspeed_sdhci,
+ width == MMC_BUS_WIDTH_8);
+
+ /* Set/clear 1 or 4 bit mode */
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+ if (width == MMC_BUS_WIDTH_4)
+ ctrl |= SDHCI_CTRL_4BITBUS;
+ else
+ ctrl &= ~SDHCI_CTRL_4BITBUS;
+ sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+}
+
+static const struct sdhci_ops aspeed_sdhci_ops = {
+ .set_clock = aspeed_sdhci_set_clock,
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
+ .set_bus_width = aspeed_sdhci_set_bus_width,
+ .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
+static const struct sdhci_pltfm_data aspeed_sdc_pdata = {
+ .ops = &aspeed_sdhci_ops,
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+};
+
+static int aspeed_sdhci_probe(struct platform_device *pdev)
+{
+ struct sdhci_pltfm_host *pltfm_host;
+ struct aspeed_sdhci *dev;
+ struct sdhci_host *host;
+ u32 slot;
+ int ret;
+
+ host = sdhci_pltfm_init(pdev, &aspeed_sdc_pdata, sizeof(*dev));
+ if (IS_ERR(host))
+ return PTR_ERR(host);
+
+ pltfm_host = sdhci_priv(host);
+ dev = sdhci_pltfm_priv(pltfm_host);
+ dev->parent = dev_get_drvdata(pdev->dev.parent);
+
+ ret = of_property_read_u32(pdev->dev.of_node, "slot", &slot);
+ if (ret < 0)
+ return ret;
+ else if (slot > 2)
+ return -EINVAL;
+
+ dev->width_mask = !slot ? ASPEED_SDC_S0MMC8 : ASPEED_SDC_S1MMC8;
+
+ sdhci_get_of_property(pdev);
+
+ pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pltfm_host->clk))
+ return PTR_ERR(pltfm_host->clk);
+
+ ret = clk_prepare_enable(pltfm_host->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable SDIO clock\n");
+ goto err_pltfm_free;
+ }
+
+ 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:
+ clk_disable_unprepare(pltfm_host->clk);
+err_pltfm_free:
+ sdhci_pltfm_free(pdev);
+ return ret;
+}
+
+static int aspeed_sdhci_remove(struct platform_device *pdev)
+{
+ struct sdhci_pltfm_host *pltfm_host;
+ struct sdhci_host *host;
+ int dead;
+
+ host = platform_get_drvdata(pdev);
+ pltfm_host = sdhci_priv(host);
+
+ dead = readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff;
+
+ sdhci_remove_host(host, dead);
+
+ clk_disable_unprepare(pltfm_host->clk);
+
+ sdhci_pltfm_free(pdev);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_sdhci_of_match[] = {
+ { .compatible = "aspeed,ast2400-sdhci", },
+ { .compatible = "aspeed,ast2500-sdhci", },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, aspeed_sdhci_of_match);
+
+static struct platform_driver aspeed_sdhci_driver = {
+ .driver = {
+ .name = "sdhci-aspeed",
+ .of_match_table = aspeed_sdhci_of_match,
+ },
+ .probe = aspeed_sdhci_probe,
+ .remove = aspeed_sdhci_remove,
+};
+
+module_platform_driver(aspeed_sdhci_driver);
+
+static int aspeed_sdc_probe(struct platform_device *pdev)
+
+{
+ struct device_node *parent, *child;
+ struct aspeed_sdc *sdc;
+ int ret;
+
+ sdc = devm_kzalloc(&pdev->dev, sizeof(*sdc), GFP_KERNEL);
+ if (!sdc)
+ return -ENOMEM;
+
+ spin_lock_init(&sdc->lock);
+
+ sdc->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(sdc->clk))
+ return PTR_ERR(sdc->clk);
+
+ ret = clk_prepare_enable(sdc->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable SDCLK\n");
+ return ret;
+ }
+
+ sdc->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(sdc->regs)) {
+ ret = PTR_ERR(sdc->regs);
+ goto err_clk;
+ }
+
+ dev_set_drvdata(&pdev->dev, sdc);
+
+ parent = pdev->dev.of_node;
+ for_each_available_child_of_node(parent, child) {
+ struct platform_device *cpdev;
+
+ cpdev = of_platform_device_create(child, NULL, &pdev->dev);
+ if (IS_ERR(cpdev)) {
+ of_node_put(child);
+ ret = PTR_ERR(pdev);
+ goto err_clk;
+ }
+ }
+
+ return 0;
+
+err_clk:
+ clk_disable_unprepare(sdc->clk);
+ return ret;
+}
+
+static int aspeed_sdc_remove(struct platform_device *pdev)
+{
+ struct aspeed_sdc *sdc = dev_get_drvdata(&pdev->dev);
+
+ clk_disable_unprepare(sdc->clk);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_sdc_of_match[] = {
+ { .compatible = "aspeed,ast2400-sdc", .data = &aspeed_sdc_pdata },
+ { .compatible = "aspeed,ast2500-sdc", .data = &aspeed_sdc_pdata },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, aspeed_sdc_of_match);
+
+static struct platform_driver aspeed_sdc_driver = {
+ .driver = {
+ .name = "sdc-aspeed",
+ .pm = &sdhci_pltfm_pmops,
+ .of_match_table = aspeed_sdc_of_match,
+ },
+ .probe = aspeed_sdc_probe,
+ .remove = aspeed_sdc_remove,
+};
+
+module_platform_driver(aspeed_sdc_driver);
+
+MODULE_DESCRIPTION("Driver for the ASPEED SD/SDIO/SDHCI Controllers");
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Andrew Jeffery @ 2019-07-10 14:16 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141611.21159-1-andrew@aj.id.au>
The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the
SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
data bus if only a single slot is enabled.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
.../bindings/mmc/sdhci-of-aspeed.yaml | 91 +++++++++++++++++++
1 file changed, 91 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
new file mode 100644
index 000000000000..e98a2ac4d46d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/sdhci-of-aspeed.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED SD/SDIO/eMMC Controller
+
+maintainers:
+ - Andrew Jeffery <andrew@aj.id.au>
+ - Ryan Chen <ryanchen.aspeed@gmail.com>
+
+description: |+
+ The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO
+ Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if
+ only a single slot is enabled.
+
+ The two slots are supported by a common configuration area. As the SDHCIs for
+ the slots are dependent on the common configuration area, they are described
+ as child nodes.
+
+properties:
+ compatible:
+ enum: [ aspeed,ast2400-sdc, aspeed,ast2500-sdc ]
+ reg:
+ description: Common configuration registers
+ ranges: true
+ clocks:
+ maxItems: 1
+ description: The SD/SDIO controller clock gate
+ sdhci:
+ type: object
+ properties:
+ compatible:
+ allOf:
+ - enum: [ aspeed,ast2400-sdhci, aspeed,ast2500-sdhci ]
+ - const: sdhci
+ reg:
+ description: The SDHCI registers
+ clocks:
+ maxItems: 1
+ description: The SD bus clock
+ slot:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - enum: [0, 1]
+ interrupts:
+ maxItems: 1
+ description: The SD interrupt shared between both slots
+ required:
+ - compatible
+ - reg
+ - clocks
+ - slot
+ - interrupts
+
+required:
+ - compatible
+ - reg
+ - ranges
+ - clocks
+
+examples:
+ - |
+ #include <dt-bindings/clock/aspeed-clock.h>
+ sdc at 1e740000 {
+ compatible = "aspeed,ast2500-sdc";
+ reg = <0x1e740000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
+
+ sdhci0: sdhci at 1e740100 {
+ compatible = "aspeed,ast2500-sdhci", "sdhci";
+ reg = <0x1e740100 0x100>;
+ slot = <0>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ };
+
+ sdhci1: sdhci at 1e740200 {
+ compatible = "aspeed,ast2500-sdhci", "sdhci";
+ reg = <0x1e740200 0x100>;
+ slot = <1>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ };
+ };
--
2.20.1
^ permalink raw reply related
* [PATCH 0/2] mmc: Add support for the ASPEED SD controller
From: Andrew Jeffery @ 2019-07-10 14:16 UTC (permalink / raw)
To: linux-aspeed
Hello,
This short series introduce devicetree bindings and a driver for the ASPEED SD
controller. Please review!
Andrew
Andrew Jeffery (2):
dt-bindings: mmc: Document Aspeed SD controller
mmc: Add support for the ASPEED SD controller
.../bindings/mmc/sdhci-of-aspeed.yaml | 91 ++++++
drivers/mmc/host/Kconfig | 12 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-of-aspeed.c | 307 ++++++++++++++++++
4 files changed, 411 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-aspeed.yaml
create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c
--
2.20.1
^ permalink raw reply
* [PATCH 3/3] ARM: dts: aspeed: Enable both MMC slots on Swift
From: Andrew Jeffery @ 2019-07-10 14:15 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141503.21026-1-andrew@aj.id.au>
From: Joel Stanley <joel@jms.id.au>
Swift will provide at least its rootfs on eMMC.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
.../boot/dts/aspeed-bmc-opp-witherspoon.dts | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index f1356ca794d8..fcaeae2dd0a8 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -640,3 +640,21 @@
&vhub {
status = "okay";
};
+
+&sdc {
+ status = "okay";
+};
+
+&sdhci0 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sd1_default>;
+};
+
+&sdhci1 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sd2_default>;
+};
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] ARM: dts: aspeed: Enable both MMC slots on AST2500 EVB
From: Andrew Jeffery @ 2019-07-10 14:15 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141503.21026-1-andrew@aj.id.au>
Enabled for testing purposes.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 556ed469830c..429904e401ee 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -94,6 +94,24 @@
};
};
+&sdc {
+ status = "okay";
+};
+
+&sdhci0 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sd1_default>;
+};
+
+&sdhci1 {
+ 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
--
2.20.1
^ permalink raw reply related
* [PATCH 1/3] ARM: dts: aspeed: Describe SD controller in DTSIs
From: Andrew Jeffery @ 2019-07-10 14:15 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141503.21026-1-andrew@aj.id.au>
The AST2400 and AST2500 both share the same SD controller, at the same
location in the physical address space and the same hardware interrupt,
with the same clock configurations.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/boot/dts/aspeed-g4.dtsi | 30 ++++++++++++++++++++++++++++++
arch/arm/boot/dts/aspeed-g5.dtsi | 30 ++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 5d7050d00874..4bfda5d91dbe 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -188,6 +188,36 @@
reg = <0x1e720000 0x8000>; // 32K
};
+ sdc: sdc at 1e740000 {
+ compatible = "aspeed,ast2400-sdc";
+ reg = <0x1e740000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
+ status = "disabled";
+
+ sdhci0: sdhci at 1e740100 {
+ compatible = "aspeed,ast2400-sdhci", "sdhci";
+ reg = <0x1e740100 0x100>;
+ aspeed,sdhci-slot = <0>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ status = "disabled";
+ };
+
+ sdhci1: sdhci at 1e740200 {
+ compatible = "aspeed,ast2400-sdhci", "sdhci";
+ reg = <0x1e740200 0x100>;
+ aspeed,sdhci-slot = <1>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ status = "disabled";
+ };
+ };
+
gpio: gpio at 1e780000 {
#gpio-cells = <2>;
gpio-controller;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 4345c3153ca7..8d6404311652 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -262,6 +262,36 @@
reg = <0x1e720000 0x9000>; // 36K
};
+ sdc: sdc at 1e740000 {
+ compatible = "aspeed,ast2500-sdc";
+ reg = <0x1e740000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
+ status = "disabled";
+
+ sdhci0: sdhci at 1e740100 {
+ compatible = "aspeed,ast2500-sdhci", "sdhci";
+ reg = <0x1e740100 0x100>;
+ slot = <0>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ status = "disabled";
+ };
+
+ sdhci1: sdhci at 1e740200 {
+ compatible = "aspeed,ast2500-sdhci", "sdhci";
+ reg = <0x1e740200 0x100>;
+ slot = <1>;
+ interrupts = <26>;
+ sdhci,auto-cmd12;
+ clocks = <&syscon ASPEED_CLK_SDIO>;
+ status = "disabled";
+ };
+ };
+
gpio: gpio at 1e780000 {
#gpio-cells = <2>;
gpio-controller;
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] ARM: dts: aspeed: Enable SD controllers
From: Andrew Jeffery @ 2019-07-10 14:15 UTC (permalink / raw)
To: linux-aspeed
Hello,
This series describes the ASPEED SD controller in relevant devicetree files,
enabling the MMC slots on the AST2500 EVB and Swift machines.
Please review!
Andrew
Andrew Jeffery (2):
ARM: dts: aspeed: Describe SD controller in DTSIs
ARM: dts: aspeed: Enable both MMC slots on AST2500 EVB
Joel Stanley (1):
ARM: dts: aspeed: Enable both MMC slots on Swift
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 18 +++++++++++
.../boot/dts/aspeed-bmc-opp-witherspoon.dts | 18 +++++++++++
arch/arm/boot/dts/aspeed-g4.dtsi | 30 +++++++++++++++++++
arch/arm/boot/dts/aspeed-g5.dtsi | 30 +++++++++++++++++++
4 files changed, 96 insertions(+)
--
2.20.1
^ permalink raw reply
* [PATCH 3/3] ARM: config: aspeed-g5: Enable EXT4, VFAT
From: Andrew Jeffery @ 2019-07-10 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141325.20888-1-andrew@aj.id.au>
From: Joel Stanley <joel@jms.id.au>
Popular filesystems now that we have eMMC support.
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/configs/aspeed_g5_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 9cb3cfd35a59..13db73e15539 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -217,8 +217,11 @@ CONFIG_FSI_MASTER_HUB=y
CONFIG_FSI_MASTER_AST_CF=y
CONFIG_FSI_SCOM=y
CONFIG_FSI_SBEFIFO=y
+CONFIG_EXT4_FS=y
CONFIG_FANOTIFY=y
CONFIG_OVERLAY_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_FAT_DEFAULT_UTF8=y
CONFIG_TMPFS=y
CONFIG_JFFS2_FS=y
# CONFIG_JFFS2_FS_WRITEBUFFER is not set
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] ARM: config: aspeed-g5: Enable SD Controller
From: Andrew Jeffery @ 2019-07-10 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141325.20888-1-andrew@aj.id.au>
From: Ryan Chen <ryanchen.aspeed@gmail.com>
Enable various options necessary for building the driver for the ASPEED
SD controller.
Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/configs/aspeed_g5_defconfig | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 249eeeb55d59..9cb3cfd35a59 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -182,6 +182,13 @@ 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_DEBUG=y
+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.20.1
^ permalink raw reply related
* [PATCH 1/3] ARM: config: aspeed-g5: Refresh on 5.2
From: Andrew Jeffery @ 2019-07-10 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190710141325.20888-1-andrew@aj.id.au>
$ make aspeed_g5_defconfig &&
make savedefconfig &&
mv defconfig arch/arm/configs/aspeed_g5_defconfig
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/configs/aspeed_g5_defconfig | 54 ++++++++++++----------------
1 file changed, 22 insertions(+), 32 deletions(-)
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 407ffb7655a8..249eeeb55d59 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -20,29 +20,28 @@ CONFIG_PERF_EVENTS=y
# CONFIG_COMPAT_BRK is not set
CONFIG_SLAB=y
CONFIG_SLAB_FREELIST_RANDOM=y
-CONFIG_JUMP_LABEL=y
-CONFIG_STRICT_KERNEL_RWX=y
-CONFIG_GCC_PLUGINS=y
-# CONFIG_BLK_DEV_BSG is not set
-# CONFIG_BLK_DEBUG_FS is not set
-# CONFIG_IOSCHED_DEADLINE is not set
-# CONFIG_MQ_IOSCHED_DEADLINE is not set
-# CONFIG_MQ_IOSCHED_KYBER is not set
CONFIG_ARCH_MULTI_V6=y
# CONFIG_ARCH_MULTI_V7 is not set
CONFIG_ARCH_ASPEED=y
CONFIG_MACH_ASPEED_G5=y
# CONFIG_CACHE_L2X0 is not set
CONFIG_VMSPLIT_2G=y
-# CONFIG_COMPACTION is not set
CONFIG_UACCESS_WITH_MEMCPY=y
CONFIG_SECCOMP=y
# CONFIG_ATAGS is not set
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
CONFIG_KEXEC=y
-# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
# CONFIG_SUSPEND is not set
+CONFIG_FIRMWARE_MEMMAP=y
+CONFIG_JUMP_LABEL=y
+CONFIG_STRICT_KERNEL_RWX=y
+# CONFIG_BLK_DEV_BSG is not set
+# CONFIG_BLK_DEBUG_FS is not set
+# CONFIG_MQ_IOSCHED_DEADLINE is not set
+# CONFIG_MQ_IOSCHED_KYBER is not set
+# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
+# CONFIG_COMPACTION is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_PACKET_DIAG=y
@@ -51,20 +50,12 @@ CONFIG_UNIX_DIAG=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_SYN_COOKIES=y
-# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
-# CONFIG_INET_XFRM_MODE_TUNNEL is not set
-# CONFIG_INET_XFRM_MODE_BEET is not set
# CONFIG_INET_DIAG is not set
-# CONFIG_INET6_XFRM_MODE_TRANSPORT is not set
-# CONFIG_INET6_XFRM_MODE_TUNNEL is not set
-# CONFIG_INET6_XFRM_MODE_BEET is not set
CONFIG_NETFILTER=y
# CONFIG_NETFILTER_ADVANCED is not set
CONFIG_VLAN_8021Q=y
CONFIG_NET_NCSI=y
-CONFIG_BPF_STREAM_PARSER=y
# CONFIG_WIRELESS is not set
-CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
@@ -78,8 +69,6 @@ CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_FASTMAP=y
CONFIG_MTD_UBI_BLOCK=y
CONFIG_BLK_DEV_LOOP=y
-CONFIG_ASPEED_LPC_CTRL=y
-CONFIG_ASPEED_LPC_SNOOP=y
CONFIG_EEPROM_AT24=y
CONFIG_NETDEVICES=y
CONFIG_NETCONSOLE=y
@@ -209,6 +198,8 @@ CONFIG_RTC_DRV_PCF8523=y
CONFIG_RTC_DRV_RV8803=y
# CONFIG_VIRTIO_MENU is not set
# CONFIG_IOMMU_SUPPORT is not set
+CONFIG_ASPEED_LPC_CTRL=y
+CONFIG_ASPEED_LPC_SNOOP=y
CONFIG_IIO=y
CONFIG_ASPEED_ADC=y
CONFIG_MAX1363=y
@@ -219,7 +210,6 @@ CONFIG_FSI_MASTER_HUB=y
CONFIG_FSI_MASTER_AST_CF=y
CONFIG_FSI_SCOM=y
CONFIG_FSI_SBEFIFO=y
-CONFIG_FIRMWARE_MEMMAP=y
CONFIG_FANOTIFY=y
CONFIG_OVERLAY_FS=y
CONFIG_TMPFS=y
@@ -232,6 +222,17 @@ CONFIG_SQUASHFS=y
CONFIG_SQUASHFS_XZ=y
CONFIG_SQUASHFS_ZSTD=y
# CONFIG_NETWORK_FILESYSTEMS is not set
+CONFIG_HARDENED_USERCOPY=y
+CONFIG_FORTIFY_SOURCE=y
+# CONFIG_CRYPTO_ECHAINIV is not set
+CONFIG_CRYPTO_HMAC=y
+CONFIG_CRYPTO_SHA256=y
+CONFIG_CRYPTO_USER_API_HASH=y
+# CONFIG_CRYPTO_HW is not set
+# CONFIG_XZ_DEC_X86 is not set
+# CONFIG_XZ_DEC_POWERPC is not set
+# CONFIG_XZ_DEC_IA64 is not set
+# CONFIG_XZ_DEC_SPARC is not set
CONFIG_PRINTK_TIME=y
CONFIG_DYNAMIC_DEBUG=y
CONFIG_DEBUG_INFO=y
@@ -250,14 +251,3 @@ CONFIG_FUNCTION_TRACER=y
# CONFIG_RUNTIME_TESTING_MENU is not set
CONFIG_DEBUG_WX=y
CONFIG_DEBUG_USER=y
-CONFIG_HARDENED_USERCOPY=y
-CONFIG_FORTIFY_SOURCE=y
-# CONFIG_CRYPTO_ECHAINIV is not set
-CONFIG_CRYPTO_HMAC=y
-CONFIG_CRYPTO_SHA256=y
-CONFIG_CRYPTO_USER_API_HASH=y
-# CONFIG_CRYPTO_HW is not set
-# CONFIG_XZ_DEC_X86 is not set
-# CONFIG_XZ_DEC_POWERPC is not set
-# CONFIG_XZ_DEC_IA64 is not set
-# CONFIG_XZ_DEC_SPARC is not set
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] ARM: configs: aspeed-g5: SD Controller support
From: Andrew Jeffery @ 2019-07-10 14:13 UTC (permalink / raw)
To: linux-aspeed
Hello,
This series adds SD controller related bits to the aspeed_g5_defconfig.
Please review!
Andrew
Andrew Jeffery (1):
ARM: config: aspeed-g5: Refresh on 5.2
Joel Stanley (1):
ARM: config: aspeed-g5: Enable EXT4, VFAT
Ryan Chen (1):
ARM: config: aspeed-g5: Enable SD Controller
arch/arm/configs/aspeed_g5_defconfig | 64 ++++++++++++++--------------
1 file changed, 32 insertions(+), 32 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH] clk: aspeed: Add SDIO gate
From: Andrew Jeffery @ 2019-07-10 14:10 UTC (permalink / raw)
To: linux-aspeed
From: Joel Stanley <joel@jms.id.au>
The clock divisor comes with an enable bit (gate). This was not
implemented as we didn't have access to SD hardware when writing the
driver. Now that we can test it, add the gate as a parent to the
divisor.
There is no reason to expose the gate separately, so users will enable
it by turning on the ASPEED_CLK_SDIO divisor.
Signed-off-by: Joel Stanley <joel@jms.id.au>
[aj: Minor style cleanup]
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/clk/clk-aspeed.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 42b4df6ba249..898291501f45 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -500,9 +500,14 @@ static int aspeed_clk_probe(struct platform_device *pdev)
return PTR_ERR(hw);
aspeed_clk_data->hws[ASPEED_CLK_MPLL] = hw;
- /* SD/SDIO clock divider (TODO: There's a gate too) */
- hw = clk_hw_register_divider_table(dev, "sdio", "hpll", 0,
- scu_base + ASPEED_CLK_SELECTION, 12, 3, 0,
+ /* SD/SDIO clock divider and gate */
+ hw = clk_hw_register_gate(dev, "sd_extclk_gate", "hpll", 0,
+ scu_base + ASPEED_CLK_SELECTION, 15, 0,
+ &aspeed_clk_lock);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
+ 0, scu_base + ASPEED_CLK_SELECTION, 12, 3, 0,
soc_data->div_table,
&aspeed_clk_lock);
if (IS_ERR(hw))
--
2.20.1
^ permalink raw reply related
* [PATCH 00/12] treewide: Fix GENMASK misuses
From: Russell King - ARM Linux admin @ 2019-07-10 9:43 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <5fa1fa6998332642c49e2d5209193ffe2713f333.camel@sipsolutions.net>
On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote:
> On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> > These GENMASK uses are inverted argument order and the
> > actual masks produced are incorrect. Fix them.
> >
> > Add checkpatch tests to help avoid more misuses too.
> >
> > Joe Perches (12):
> > checkpatch: Add GENMASK tests
>
> IMHO this doesn't make a lot of sense as a checkpatch test - just throw
> in a BUILD_BUG_ON()?
My personal take on this is that GENMASK() is really not useful, it's
just pure obfuscation and leads to exactly these kinds of mistakes.
Yes, I fully understand the argument that you can just specify the
start and end bits, and it _in theory_ makes the code more readable.
However, the problem is when writing code. GENMASK(a, b). Is a the
starting bit or ending bit? Is b the number of bits? It's confusing
and causes mistakes resulting in incorrect code. A BUILD_BUG_ON()
can catch some of the cases, but not all of them.
For example:
GENMASK(6, 2)
would satisify the requirement that a > b, so a BUILD_BUG_ON() will
not trigger, but was the author meaning 0x3c or 0xc0?
Personally, I've decided I am _not_ going to use GENMASK() in my code
because I struggle to get the macro arguments correct - I'm _much_
happier, and it is way more reliable for me to write the mask in hex
notation.
I think this is where use of a ternary operator would come in use. The
normal way of writing a number of bits tends to be "a:b", so if GENMASK
took something like GENMASK(6:2), then I'd have less issue with it,
because it's argument is then in a familiar notation.
Yes, I'm sure that someone will point out that the GENMASK arguments
are just in the same order, but that doesn't prevent _me_ frequently
getting it wrong - and that's the point. The macro seems to me to
cause more problems than it solves.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH 00/12] treewide: Fix GENMASK misuses
From: Johannes Berg @ 2019-07-10 9:17 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <cover.1562734889.git.joe@perches.com>
On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
> These GENMASK uses are inverted argument order and the
> actual masks produced are incorrect. Fix them.
>
> Add checkpatch tests to help avoid more misuses too.
>
> Joe Perches (12):
> checkpatch: Add GENMASK tests
IMHO this doesn't make a lot of sense as a checkpatch test - just throw
in a BUILD_BUG_ON()?
johannes
^ permalink raw reply
* [PATCH 03/12] drm: aspeed_gfx: Fix misuse of GENMASK macro
From: Joe Perches @ 2019-07-10 5:04 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <cover.1562734889.git.joe@perches.com>
Arguments are supposed to be ordered high then low.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/gpu/drm/aspeed/aspeed_gfx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h
index a10358bb61ec..095ea03e5833 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx.h
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h
@@ -74,7 +74,7 @@ int aspeed_gfx_create_output(struct drm_device *drm);
/* CTRL2 */
#define CRT_CTRL_DAC_EN BIT(0)
#define CRT_CTRL_VBLANK_LINE(x) (((x) << 20) & CRT_CTRL_VBLANK_LINE_MASK)
-#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(20, 31)
+#define CRT_CTRL_VBLANK_LINE_MASK GENMASK(31, 20)
/* CRT_HORIZ0 */
#define CRT_H_TOTAL(x) (x)
--
2.15.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox