| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54F97C27.508@redhat.com>
Date: Fri, 06 Mar 2015 11:06:31 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Aleksey Makarov <aleksey.makarov@...iga.com>,
linux-ide@...r.kernel.org
CC: linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org,
David Daney <david.daney@...ium.com>,
Vinita Gupta <vgupta@...iumnetworks.com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>, Tejun Heo <tj@...nel.org>,
devicetree@...r.kernel.org
Subject: Re: [PATCH v3] SATA: OCTEON: support SATA on OCTEON platform
Hi,
On 05-03-15 15:58, Aleksey Makarov wrote:
> The OCTEON SATA controller is currently found on cn71XX devices.
>
> Signed-off-by: David Daney <david.daney@...ium.com>
> Signed-off-by: Vinita Gupta <vgupta@...iumnetworks.com>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@...iga.com>
> ---
>
> Version 2:
> https://lkml.kernel.org/g/<1422038495-5204-1-git-send-email-aleksey.makarov@...iga.com>
>
> Changes in v3:
> - Rebased to v4.0-rc2
> - Cosmetic changes
>
> Changes in v2:
> - The driver was rewritten as a driver for the UCTL SATA controller glue.
> It allowed to get rid of the most changes in ahci_platform.c
> - Documentation for the device tree bindings was fixed.
>
> .../devicetree/bindings/ata/ahci-platform.txt | 1 +
> .../devicetree/bindings/mips/cavium/sata-uctl.txt | 28 ++++
> drivers/ata/Kconfig | 9 ++
> drivers/ata/Makefile | 1 +
> drivers/ata/ahci_platform.c | 1 +
> drivers/ata/sata_octeon.c | 157 +++++++++++++++++++++
> 6 files changed, 197 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mips/cavium/sata-uctl.txt
> create mode 100644 drivers/ata/sata_octeon.c
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index c2340ee..3d84dca 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -11,6 +11,7 @@ Required properties:
> - compatible : compatible string, one of:
> - "allwinner,sun4i-a10-ahci"
> - "hisilicon,hisi-ahci"
> + - "cavium,octeon-7130-ahci"
> - "ibm,476gtr-ahci"
> - "marvell,armada-380-ahci"
> - "snps,dwc-ahci"
> diff --git a/Documentation/devicetree/bindings/mips/cavium/sata-uctl.txt b/Documentation/devicetree/bindings/mips/cavium/sata-uctl.txt
> new file mode 100644
> index 0000000..59e86a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/cavium/sata-uctl.txt
> @@ -0,0 +1,28 @@
> +* UCTL SATA controller glue
> +
> +Properties:
> +- compatible: "cavium,octeon-7130-sata-uctl"
> +
> + Compatibility with the cn7130 SOC.
> +
> +- reg: The base address of the UCTL register bank.
> +
> +- #address-cells, #size-cells, and ranges must be present and hold
> + suitable values to map all child nodes.
> +
> +Example:
> +
> + uctl@...006c000000 {
> + compatible = "cavium,octeon-7130-sata-uctl";
> + reg = <0x11800 0x6c000000 0x0 0x100>;
> + ranges; /* Direct mapping */
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + sata: sata@...0000000000 {
> + compatible = "cavium,octeon-7130-ahci";
> + reg = <0x16c00 0x00000000 0x0 0x200>;
> + interrupt-parent = <&cibsata>;
> + interrupts = <2 4>; /* Bit: 2, level */
> + };
> + };
Sorry for jumping into this discussion a bit late, but this nonsense
nesting of what clearly are 2 related but different hw blocks,
both living at completely different register addresses is unacceptable,
this is not a proper operating system dependent hw description as
devicetree is supposed to be. This is an ugly hack to ensure a
certain init ordering, and requiring manual instantiation of
the platform device for the nested dt-node.
NACK.
The proper solution for this would be to have a single sata node,
with 2 register ranges, the first one for the actual ahci block,
and the second range for the uctl glue, and then write your
own ahci_octeon.c platform driver (you can start with a copy of
ahci_platform.c) which can do the uctl poking in its probe function
before calling ahci_platform_get_resources,
ahci_platform_enable_resources and ahci_platform_init_host.
Minus the standard boilerplate ahci_platform.c is only 60 lines,
it is THAT small because we've put all the common ahci-platform
code into libahci_platform.c exactly so that it is trivial to
create new ahci-platform drivers for special cases like this,
so that we do not need to do these kind of ugly hacks.
Actually libahciplatform was specifically written to remove a couple
of very similar hacks with platform device drivers instantiating
more platform devices from the kernel ...
Regards,
Hans
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 5f60155..55ad870 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -188,6 +188,15 @@ config SATA_SIL24
>
> If unsure, say N.
>
> +config SATA_OCTEON
> + tristate "Cavium Octeon Soc Serial ATA"
> + depends on SATA_AHCI_PLATFORM && CAVIUM_OCTEON_SOC
> + default y
> + help
> + This option enables support for Cavium Octeon SoC Serial ATA.
> +
> + If unsure, say N.
> +
> config ATA_SFF
> bool "ATA SFF support (for legacy IDE and PATA)"
> default y
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index ae41107..4a0e5e3 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_AHCI_SUNXI) += ahci_sunxi.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_ST) += ahci_st.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_TEGRA) += ahci_tegra.o libahci.o libahci_platform.o
> obj-$(CONFIG_AHCI_XGENE) += ahci_xgene.o libahci.o libahci_platform.o
> +obj-$(CONFIG_SATA_OCTEON) += sata_octeon.o
>
> # SFF w/ custom DMA
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 78d6ae0..2c26cde 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -74,6 +74,7 @@ static const struct of_device_id ahci_of_match[] = {
> { .compatible = "ibm,476gtr-ahci", },
> { .compatible = "snps,dwc-ahci", },
> { .compatible = "hisilicon,hisi-ahci", },
> + { .compatible = "cavium,octeon-7130-ahci", },
> {},
> };
> MODULE_DEVICE_TABLE(of, ahci_of_match);
> diff --git a/drivers/ata/sata_octeon.c b/drivers/ata/sata_octeon.c
> new file mode 100644
> index 0000000..338ea86
> --- /dev/null
> +++ b/drivers/ata/sata_octeon.c
> @@ -0,0 +1,157 @@
> +/*
> + * SATA glue for Cavium Octeon III SOCs.
> + *
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2010-2015 Cavium Networks
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/octeon/octeon.h>
> +#include <asm/bitfield.h>
> +
> +/**
> + * cvmx_sata_uctl_shim_cfg
> + * from cvmx-sata-defs.h
> + *
> + * Accessible by: only when A_CLKDIV_EN
> + * Reset by: IOI reset (srst_n) or SATA_UCTL_CTL[SATA_UCTL_RST]
> + * This register allows configuration of various shim (UCTL) features.
> + * Fields XS_NCB_OOB_* are captured when there are no outstanding OOB errors
> + * indicated in INTSTAT and a new OOB error arrives.
> + * Fields XS_BAD_DMA_* are captured when there are no outstanding DMA errors
> + * indicated in INTSTAT and a new DMA error arrives.
> + */
> +union cvmx_sata_uctl_shim_cfg {
> + uint64_t u64;
> + struct cvmx_sata_uctl_shim_cfg_s {
> + /*
> + * Read/write error log for out-of-bound UAHC register access.
> + * 0 = read, 1 = write.
> + */
> + __BITFIELD_FIELD(uint64_t xs_ncb_oob_wrn : 1,
> + __BITFIELD_FIELD(uint64_t reserved_57_62 : 6,
> + /*
> + * SRCID error log for out-of-bound UAHC register access.
> + * The IOI outbound SRCID for the OOB error.
> + */
> + __BITFIELD_FIELD(uint64_t xs_ncb_oob_osrc : 9,
> + /*
> + * Read/write error log for bad DMA access from UAHC.
> + * 0 = read error log, 1 = write error log.
> + */
> + __BITFIELD_FIELD(uint64_t xm_bad_dma_wrn : 1,
> + __BITFIELD_FIELD(uint64_t reserved_44_46 : 3,
> + /*
> + * ErrType error log for bad DMA access from UAHC. Encodes the type of
> + * error encountered (error largest encoded value has priority).
> + * See SATA_UCTL_XM_BAD_DMA_TYPE_E.
> + */
> + __BITFIELD_FIELD(uint64_t xm_bad_dma_type : 4,
> + __BITFIELD_FIELD(uint64_t reserved_13_39 : 27,
> + /*
> + * Selects the IOI read command used by DMA accesses.
> + * See SATA_UCTL_DMA_READ_CMD_E.
> + */
> + __BITFIELD_FIELD(uint64_t dma_read_cmd : 1,
> + __BITFIELD_FIELD(uint64_t reserved_10_11 : 2,
> + /*
> + * Selects the endian format for DMA accesses to the L2C.
> + * See SATA_UCTL_ENDIAN_MODE_E.
> + */
> + __BITFIELD_FIELD(uint64_t dma_endian_mode : 2,
> + __BITFIELD_FIELD(uint64_t reserved_2_7 : 6,
> + /*
> + * Selects the endian format for IOI CSR accesses to the UAHC.
> + * Note that when UAHC CSRs are accessed via RSL, they are returned
> + * as big-endian. See SATA_UCTL_ENDIAN_MODE_E.
> + */
> + __BITFIELD_FIELD(uint64_t csr_endian_mode : 2,
> + ;))))))))))))
> + } s;
> +};
> +
> +#define CVMX_SATA_UCTL_SHIM_CFG 0xE8
> +
> +static int ahci_octeon_probe(struct platform_device *pdev)
> +{
> + union cvmx_sata_uctl_shim_cfg shim_cfg;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + struct resource *res;
> + void __iomem *base;
> + int ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "Platform resource[0] is missing\n");
> + return -ENODEV;
> + }
> +
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + /* set-up endian mode */
> + shim_cfg.u64 = cvmx_read_csr((uint64_t)base + CVMX_SATA_UCTL_SHIM_CFG);
> +#ifdef __BIG_ENDIAN
> + shim_cfg.s.dma_endian_mode = 1;
> + shim_cfg.s.csr_endian_mode = 1;
> +#else
> + shim_cfg.s.dma_endian_mode = 0;
> + shim_cfg.s.csr_endian_mode = 0;
> +#endif
> + shim_cfg.s.dma_read_cmd = 1; /* No allocate L2C */
> + cvmx_write_csr((uint64_t)base + CVMX_SATA_UCTL_SHIM_CFG, shim_cfg.u64);
> +
> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (ret)
> + return ret;
> +
> + if (!node) {
> + dev_err(dev, "no device node, failed to add octeon sata\n");
> + return -ENODEV;
> + }
> +
> + ret = of_platform_populate(node, NULL, NULL, dev);
> + if (ret) {
> + dev_err(dev, "failed to add ahci-platform core\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ahci_octeon_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +
> +static const struct of_device_id octeon_ahci_match[] = {
> + { .compatible = "cavium,octeon-7130-sata-uctl", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, octeon_ahci_match);
> +
> +static struct platform_driver ahci_octeon_driver = {
> + .probe = ahci_octeon_probe,
> + .remove = ahci_octeon_remove,
> + .driver = {
> + .name = "octeon-ahci",
> + .of_match_table = octeon_ahci_match,
> + },
> +};
> +
> +module_platform_driver(ahci_octeon_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Cavium, Inc. <support@...ium.com>");
> +MODULE_DESCRIPTION("Cavium Inc. sata config.");
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists