Hi Daniel!
Thank you for the input. I am about to have a new v2 ready.
Comments below.
---Lars
-----Original Message----- From: Daniel Schwierzeck daniel.schwierzeck@gmail.com Sent: Thursday, December 20, 2018 21:53 To: Lars Povlsen - M31675 Lars.Povlsen@microchip.com; u- boot@lists.denx.de Cc: gregory.clement@bootlin.com; Horatiu Vultur - M31836 Horatiu.Vultur@microchip.com Subject: Re: [PATCH 1/6] mips: mscc_sgpio: Add the MSCC serial GPIO device (SIO)
Am 20.12.18 um 21:11 schrieb Lars Povlsen:
This add support for the the MSCC serial GPIO driver in MSCC VCoreIII-based SOCs.
By using a serial interface, the SIO controller significantly extends the number of available GPIOs with a minimum number of additional pins on the device. The primary purpose of the SIO controller is to connect control signals from SFP modules and to act as an LED controller.
This adds the base driver.
[snip]
diff --git a/drivers/gpio/mscc_sgpio.c b/drivers/gpio/mscc_sgpio.c new file mode 100644 index 0000000000..28f60498fd --- /dev/null +++ b/drivers/gpio/mscc_sgpio.c @@ -0,0 +1,252 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/*
- Microsemi SoCs serial gpio driver
- Author: lars.povlsen@microchip.com
- License: Dual MIT/GPL
this line is redundant due to the SPDX identifier
Removed.
- Copyright (c) 2018 Microsemi Corporation
- */
+#include <common.h> +#include <dm.h> +#include <asm/gpio.h> +#include <asm/io.h> +#include <errno.h> +#include <clk.h>
+#define MSCC_SGPIOS_PER_BANK 32 +#define MSCC_SGPIO_BANK_DEPTH 4
[snip]
+static int mscc_sgpio_probe(struct udevice *dev) +{
- struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
- struct mscc_sgpio_priv *priv = dev_get_priv(dev);
- const void *fdt = gd->fdt_blob;
- int err, div_clock = 0, port, node = dev_of_offset(dev);
- u32 val;
- struct clk clk;
- err = clk_get_by_index(dev, 0, &clk);
- if (!err) {
err = clk_get_rate(&clk);if (!IS_ERR_VALUE(err))div_clock = err;- } else {
debug("mscc_sgpio: failed to get clock\n");return err;- }
- priv->props = (const struct mscc_sgpio_props
*)dev_get_driver_data(dev);
- priv->ports = fdtdec_get_int(fdt, node, "mscc,sgpio-ports",
0xFFFFFFFF);
- priv->clock = fdtdec_get_int(fdt, node,
"mscc,sgpio-frequency", 12500000);- if (priv->clock <= 0 || priv->clock > div_clock) {
printf("mscc_sgpio: Invalid frequency %d\n", priv->clock);return -EINVAL;- }
- priv->bitcount = fdtdec_get_int(fdt, node,
"mscc,sgpio-bitcount", 2);you should use the newer DM API from include/dm/read.h which is more readable and you don't need to poke on gd->fdt_blob anymore. This would also make your driver live-tree compatible in case you want to enable that in the future.
Now using dev_read_u32_default() instead.
- if (priv->bitcount < 1 || priv->bitcount > 4) {
printf("mscc_sgpio: bit count %d\n", priv->bitcount);return -EINVAL;- }
- priv->regs = (u32 __iomem *)devfdt_get_addr(dev);
don't you need some mapping to a virtual address? Or is this mapped through TLB? Actually the address from device tree should be a physical one.
There is a 1:1 phys to virtual TLB mapping. Also, most of the other GPIO drivers do it the same, but I added a map_physmem() call.
---Lars
- uc_priv->gpio_count = MSCC_SGPIOS_PER_BANK * priv->bitcount;
- uc_priv->bank_name = "sgpio";
- sgpio_clrsetbits(priv, REG_SIO_CONFIG, 0,
MSCC_M_CFG_SIO_PORT_WIDTH(priv),MSCC_F_CFG_SIO_PORT_WIDTH(priv, priv->bitcount - 1) |MSCC_M_CFG_SIO_AUTO_REPEAT(priv));- val = div_clock / priv->clock;
- debug("probe: div-clock = %d KHz, freq = %d KHz, div = %d\n",
div_clock / 1000, priv->clock / 1000, val);- sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0,
MSCC_M_CLOCK_SIO_CLK_FREQ(priv),MSCC_F_CLOCK_SIO_CLK_FREQ(priv, val));- for (port = 0; port < 32; port++)
sgpio_writel(priv, 0, REG_PORT_CONFIG, port);- sgpio_writel(priv, priv->ports, REG_PORT_ENABLE, 0);
- debug("probe: sgpio regs = %p\n", priv->regs);
- return 0;
+}
+static const struct dm_gpio_ops mscc_sgpio_ops = {
- .direction_input = mscc_sgpio_direction_input,
- .direction_output = mscc_sgpio_direction_output,
- .get_function = mscc_sgpio_get_function,
- .get_value = mscc_sgpio_get_value,
- .set_value = mscc_sgpio_set_value,
+};
+static const struct udevice_id mscc_sgpio_ids[] = {
- { .compatible = "mscc,luton-sgpio", .data = (ulong)&props_luton },
- { .compatible = "mscc,ocelot-sgpio", .data = (ulong)&props_ocelot
},
- { }
+};
+U_BOOT_DRIVER(gpio_mscc_sgpio) = {
- .name = "mscc-sgpio",
- .id = UCLASS_GPIO,
- .of_match = mscc_sgpio_ids,
- .ops = &mscc_sgpio_ops,
- .probe = mscc_sgpio_probe,
- .priv_auto_alloc_size = sizeof(struct mscc_sgpio_priv),
+};
--
- Daniel