* Re: [PATCH 1/3] of: overlay_adjust_phandles() - do not modify const field
From: Frank Rowand @ 2017-04-24 22:16 UTC (permalink / raw)
To: Rob Herring, Benjamin Herrenschmidt
Cc: Stephen Boyd, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+CTM4Br2t8+hcNnfrDhmkJTmUtcgZQC__t+Ppn94e9zw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04/24/17 14:40, Rob Herring wrote:
> +Ben H
>
> On Mon, Apr 24, 2017 at 1:54 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 04/24/17 09:56, Rob Herring wrote:
>>> On Mon, Apr 24, 2017 at 12:20 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>>>
>>>> When adjusting overlay phandles to apply to the live device tree, can
>>>> not modify the property value because it is type const.
>>>>
>>>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>>>> the type of struct property.value from void * to const void *. As
>>>> a result of the type change, the overlay code had compile errors
>>>> where the resolver updates phandle values.
>>>
>>> Conceptually, I prefer your first version. phandles are special and
>>> there's little reason to expose them except to generate a dts or dtb
>>> from /proc/device-tree. We could still generate the phandle file in
>>> that case, but I don't know if special casing phandle is worth it.
>>
>> The biggest thing that makes me wary about my first version is PPC
>> and Sparc. I can read their code, but do not know what the firmware
>> is feeding into the kernel, so I felt like there might be some
>> incorrect assumptions or fundamental misunderstandings that I
>> may have.
>
> I never let that stop me. ;) I guess one question is does
> "ibm,phandle" need to be exposed to userspace. Maybe Ben has some
> thoughts.
>
>> If we do remove the phandle properties from the live tree, I think
>> that phandle still needs to be exposed in /proc/device-tree
>> because that is important information for being able to understand
>> (or debug) code via reading the source. It isn't a lot code.
>>
>> One factor I was not sure of to help choose between the first version
>> and this approach is net memory size of the device tree:
>>
>> first version:
>>
>> Adds struct bin_attribute (28 bytes on 32 bit arm) to every node
>
> You could do a pointer to the struct instead.
>
>> Removes "linux,phandle" and "phandle" properties from nodes that
>> have a phandle (64 + 72 = 136 bytes)
>
> I don't think it is that high because we don't copy the property names
> and values. It's just 2x sizeof(struct property) plus whatever
> overhead each unflatten_dt_alloc has.
>
>> second version plus subsequent "linux,phandle" removal:
>>
>> Removes "linux,phandle" properties from nodes
>> that have a phandle (72 bytes)
>>
>> I do not have a feel of how many nodes have phandles in the many
>> different device trees, so I'm not sure of the end result for the
>> first version.
>
> Probably well under half. Though in theory dtc could create a phandle
> for every node.
>
>> I do not have a strong preference between my first approach and second
>> approach. But now that I have done both, a choice can be made. Let me
>> know which way you want to go and I'll respin the one you prefer.
>> For this version I'll make the change you suggested. For the first
>> version, I'll modify of_attach_mode() slightly more to remove any
>> "phandle", "linux,phandle", and "ibm,phandle" property from the node
>> before attaching it, and add the call to add the phandle sysfs
>> file: __of_add_phandle_sysfs(np);
>
> I still lean toward the former, but I guess it depends if there are
> really any size savings.
OK, I'll respin the first approach.
>
> Why would you remove properties in of_attach_mode? You would want to
> convert populate_properties() to store the phandle from the start and
> never create the properties.
For a tree that gets created by unflatten, yes, the phandle property
is never created with this patch applied. But PPC adds nodes (and
their properties) through of_attach_node(), so this is where I can avoid
copying phandle properties into the live tree.
> [...]
>
>>>> + prop = __of_find_property(overlay, "phandle", NULL);
>>>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>>>> + GFP_KERNEL);
>>>> + if (!newprop)
>>>> + return -ENOMEM;
>>>> + __of_update_property(overlay, newprop, &prop);
>>>> +
>>>> + prop = __of_find_property(overlay, "linux,phandle", NULL);
>>>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>>>> + GFP_KERNEL);
>>>
>>> There is no reason to support "linux,phandle" for overlays. That is
>>> legacy (pre ePAPR) which predates any overlays by a long time.
>>
>> I would like to have the same behavior for non-overlays as for overlays.
>> The driver is the same whether the device tree description comes from
>> the initial device tree or from an overlay.
>
> Agreed. You only need to store one of them for both base and overlays.
> You could go further and just ignore them altogether for overlays as
> we should never have any with linux,phandle only, but that doesn't
> really matter as it won't affect the memory usage.
>
> Rob
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: Add the ethernet and ethernet PHY to the cygnus core DT.
From: Andrew Lunn @ 2017-04-24 22:08 UTC (permalink / raw)
To: Eric Anholt
Cc: Florian Fainelli, Vivien Didelot, netdev-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Ray Jui,
Scott Branden, Jon Mason
In-Reply-To: <20170424215022.30382-3-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> + mdio: mdio@18002000 {
> + compatible = "brcm,iproc-mdio";
> + reg = <0x18002000 0x8>;
> + #size-cells = <1>;
> + #address-cells = <0>;
> +
> + gphy0: eth-gphy@0 {
> + reg = <0>;
> + max-speed = <1000>;
> + };
> +
> + gphy1: eth-gphy@1 {
> + reg = <1>;
> + max-speed = <1000>;
> + };
> + };
Hi Eric
Do these max-speed properties do anything useful? Is the PHY capable
of > 1Gbps?
Thanks
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] PCI: mediatek: Add Mediatek PCIe host controller support
From: Bjorn Helgaas @ 2017-04-24 22:02 UTC (permalink / raw)
To: Ryder Lee
Cc: Bjorn Helgaas, Rob Herring, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1492935543-18190-2-git-send-email-ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Hi Ryder,
Looks good, but I have a few questions below.
On Sun, Apr 23, 2017 at 04:19:02PM +0800, Ryder Lee wrote:
> Add support for the Mediatek PCIe controller which can be found
> on MT7623A/N, MT2701 and MT8521p platforms.
>
> Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/pci/host/Kconfig | 11 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-mediatek.c | 611 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 623 insertions(+)
> create mode 100644 drivers/pci/host/pcie-mediatek.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f7c1d4d..cf13b5d 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -174,6 +174,17 @@ config PCIE_ROCKCHIP
> There is 1 internal PCIe port available to support GEN2 with
> 4 slots.
>
> +config PCIE_MEDIATEK
> + bool "Mediatek PCIe Controller for MT7623 SoCs families"
> + depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
> + depends on OF
> + depends on PCI
> + select PCIEPORTBUS
> + help
> + Say Y here if you want to enable PCIe controller support on MT7623 A/N
> + series SoCs. There is a one root complex with 3 root ports available.
> + Each port supports Gen2 lane x1.
> +
> config VMD
> depends on PCI_MSI && X86_64 && SRCU
> tristate "Intel Volume Management Device Driver"
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 4d36866..265adff 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> +obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> obj-$(CONFIG_VMD) += vmd.o
>
> # The following drivers are for devices that use the generic ACPI
> diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> new file mode 100644
> index 0000000..98e84d9
> --- /dev/null
> +++ b/drivers/pci/host/pcie-mediatek.c
> @@ -0,0 +1,611 @@
> +/*
> + * PCIe host controller driver for Mediatek MT7623 SoCs families
> + *
> + * Copyright (c) 2017 MediaTek Inc.
> + * Author: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +/* PCIe shared registers */
> +#define PCIE_SYS_CFG 0x00
> +#define PCIE_INT_ENABLE 0x0c
> +#define PCIE_CFG_ADDR 0x20
> +#define PCIE_CFG_DATA 0x24
> +
> +/* PCIe per port registers */
> +#define PCIE_BAR0_SETUP 0x10
> +#define PCIE_BAR1_SETUP 0x14
> +#define PCIE_BAR0_MEM_BASE 0x18
> +#define PCIE_CLASS 0x34
> +#define PCIE_LINK_STATUS 0x50
> +
> +#define PCIE_PORT_INT_EN(x) BIT(20 + (x))
> +#define PCIE_PORT_PERST(x) BIT(1 + (x))
> +#define PCIE_PORT_LINKUP BIT(0)
> +#define PCIE_BAR_MAP_MAX GENMASK(31, 16)
> +
> +#define PCIE_BAR_ENABLE BIT(0)
> +#define PCIE_REVISION_ID BIT(0)
> +#define PCIE_CLASS_CODE (0x60400 << 8)
> +#define PCIE_CONF_REG(regn) (((regn) & GENMASK(7, 2)) | \
> + ((((regn) >> 8) & GENMASK(3, 0)) << 24))
> +#define PCIE_CONF_FUN(fun) (((fun) << 8) & GENMASK(10, 8))
> +#define PCIE_CONF_DEV(dev) (((dev) << 11) & GENMASK(15, 11))
> +#define PCIE_CONF_BUS(bus) (((bus) << 16) & GENMASK(23, 16))
> +#define PCIE_CONF_ADDR(regn, fun, dev, bus) \
> + (PCIE_CONF_REG(regn) | PCIE_CONF_FUN(fun) | \
> + PCIE_CONF_DEV(dev) | PCIE_CONF_BUS(bus))
> +
> +/* Mediatek specific configuration registers */
> +#define PCIE_FTS_NUM 0x70c
> +#define PCIE_FTS_NUM_MASK GENMASK(15, 8)
> +#define PCIE_FTS_NUM_L0(x) ((x) & 0xff << 8)
> +
> +#define PCIE_FC_CREDIT 0x73c
> +#define PCIE_FC_CREDIT_MASK (GENMASK(31, 31) | GENMASK(28, 16))
> +#define PCIE_FC_CREDIT_VAL(x) ((x) << 16)
> +
> +/**
> + * struct mtk_pcie_port - PCIe port information
> + * @dev: pointer to root port device
> + * @base: IO mapped register base
> + * @list: port list
> + * @pcie: pointer to PCIe host info
> + * @reset: pointer to RC reset control
> + * @regs: port memory region
> + * @sys_ck: root port clock
> + * @phy: pointer to phy control block
> + * @irq: IRQ number
> + * @lane: lane count
> + * @index: port index
> + */
> +struct mtk_pcie_port {
> + struct device *dev;
> + void __iomem *base;
> + struct list_head list;
> + struct mtk_pcie *pcie;
> + struct reset_control *reset;
> + struct resource regs;
> + struct clk *sys_ck;
> + struct phy *phy;
> + int irq;
> + u32 lane;
> + u32 index;
> +};
> +
> +/**
> + * struct mtk_pcie - PCIe host information
> + * @dev: pointer to PCIe device
> + * @base: IO mapped register Base
> + * @free_ck: free-run reference clock
> + * @resources: bus resources
> + * @ports: pointer to PCIe port information
> + */
> +struct mtk_pcie {
> + struct device *dev;
> + void __iomem *base;
> + struct clk *free_ck;
> + struct list_head resources;
> + struct list_head ports;
> +};
> +
> +static inline bool mtk_pcie_link_is_up(struct mtk_pcie_port *port)
> +{
> + return !!(readl_relaxed(port->base + PCIE_LINK_STATUS) &
> + PCIE_PORT_LINKUP);
> +}
> +
> +static bool mtk_pcie_valid_device(struct mtk_pcie *pcie,
> + struct pci_bus *bus, int devfn)
> +{
> + struct mtk_pcie_port *port;
> + struct pci_dev *dev;
> + struct pci_bus *pbus;
> +
> + /* if there is no link, then there is no device */
> + list_for_each_entry(port, &pcie->ports, list) {
> + if (bus->number == 0 && port->index == PCI_SLOT(devfn) &&
> + mtk_pcie_link_is_up(port)) {
> + return true;
> + } else if (bus->number != 0) {
> + pbus = bus;
> + do {
> + dev = pbus->self;
> + if (port->index == PCI_SLOT(dev->devfn) &&
> + mtk_pcie_link_is_up(port)) {
> + return true;
> + }
> + pbus = dev->bus;
> + } while (dev->bus->number != 0);
> + }
> + }
> +
> + return false;
> +}
> +
> +static void mtk_pcie_port_free(struct mtk_pcie_port *port)
> +{
> + struct mtk_pcie *pcie = port->pcie;
> + struct device *dev = pcie->dev;
> +
> + devm_iounmap(dev, port->base);
> + devm_release_mem_region(dev, port->regs.start,
> + resource_size(&port->regs));
> + list_del(&port->list);
> + devm_kfree(dev, port);
> +}
> +
> +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie *pcie, u32 bus, u32 devfn,
> + int where, int size, u32 *val)
> +{
> + writel(PCIE_CONF_ADDR(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> + pcie->base + PCIE_CFG_ADDR);
> +
> + *val = 0;
> +
> + switch (size) {
> + case 1:
> + *val = readb(pcie->base + PCIE_CFG_DATA + (where & 3));
> + break;
> + case 2:
> + *val = readw(pcie->base + PCIE_CFG_DATA + (where & 2));
> + break;
> + case 4:
> + *val = readl(pcie->base + PCIE_CFG_DATA);
> + break;
> + }
> +
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int mtk_pcie_hw_wr_cfg(struct mtk_pcie *pcie, u32 bus, u32 devfn,
> + int where, int size, u32 val)
> +
> +{
> + writel(PCIE_CONF_ADDR(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> + pcie->base + PCIE_CFG_ADDR);
> +
> + switch (size) {
> + case 1:
> + writeb(val, pcie->base + PCIE_CFG_DATA + (where & 3));
> + break;
> + case 2:
> + writew(val, pcie->base + PCIE_CFG_DATA + (where & 2));
> + break;
> + case 4:
> + writel(val, pcie->base + PCIE_CFG_DATA);
> + break;
> + }
> +
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int mtk_pcie_read_config(struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 *val)
> +{
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> +
> + if (!mtk_pcie_valid_device(pcie, bus, devfn)) {
> + *val = 0xffffffff;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }
I know there are some other drivers with the *_valid_device() pattern
in their config accessors, but I don't like it because it's racy.
It's possible for the link to be up for the test above, then go down
before the actual config access below.
Your hardware *should* do something sensible if we try to read config
space when the link is down, and ideally that would be enough that we
don't need this "valid_device()" check.
> + return mtk_pcie_hw_rd_cfg(pcie, bn, devfn, where, size, val);
> +}
> +
> +static int mtk_pcie_write_config(struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 val)
> +{
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> +
> + if (!mtk_pcie_valid_device(pcie, bus, devfn))
> + return PCIBIOS_DEVICE_NOT_FOUND;
> +
> + return mtk_pcie_hw_wr_cfg(pcie, bn, devfn, where, size, val);
> +}
> +
> +static struct pci_ops mtk_pcie_ops = {
> + .read = mtk_pcie_read_config,
> + .write = mtk_pcie_write_config,
> +};
> +
> +static void mtk_pcie_configure_rc(struct mtk_pcie_port *port)
> +{
> + struct mtk_pcie *pcie = port->pcie;
> + u32 val;
> +
> + /* enable interrupt */
> + val = readl(pcie->base + PCIE_INT_ENABLE);
> + val |= PCIE_PORT_INT_EN(port->index);
> + writel(val, pcie->base + PCIE_INT_ENABLE);
> +
> + /* map to all DDR region. We need to set it before cfg operation. */
> + writel(PCIE_BAR_MAP_MAX | PCIE_BAR_ENABLE,
> + port->base + PCIE_BAR0_SETUP);
> +
> + /* configure class Code and revision ID */
> + writel(PCIE_CLASS_CODE | PCIE_REVISION_ID,
> + port->base + PCIE_CLASS);
> +
> + /* configure FC credit */
> + mtk_pcie_hw_rd_cfg(pcie, 0, (port->index << 3),
> + PCIE_FC_CREDIT, 4, &val);
> + val &= ~PCIE_FC_CREDIT_MASK;
> + val |= PCIE_FC_CREDIT_VAL(0x806c);
> + mtk_pcie_hw_wr_cfg(pcie, 0, (port->index << 3),
> + PCIE_FC_CREDIT, 4, val);
> +
> + /* configure RC FTS number to 250 when it leaves L0s */
> + mtk_pcie_hw_rd_cfg(pcie, 0, (port->index << 3),
> + PCIE_FTS_NUM, 4, &val);
> + val &= ~PCIE_FTS_NUM_MASK;
> + val |= PCIE_FTS_NUM_L0(0x50);
> + mtk_pcie_hw_wr_cfg(pcie, 0, (port->index << 3),
> + PCIE_FTS_NUM, 4, val);
> +}
> +
> +static void mtk_pcie_assert_ports(struct mtk_pcie_port *port)
> +{
> + struct mtk_pcie *pcie = port->pcie;
> + u32 val;
> +
> + /* assert port PERST_N */
> + val = readl(pcie->base + PCIE_SYS_CFG);
> + val |= PCIE_PORT_PERST(port->index);
> + writel(val, pcie->base + PCIE_SYS_CFG);
> +
> + /* de-assert port PERST_N */
> + val = readl(pcie->base + PCIE_SYS_CFG);
> + val &= ~PCIE_PORT_PERST(port->index);
> + writel(val, pcie->base + PCIE_SYS_CFG);
> +
> + /*
> + * at least 100ms delay because PCIe v2.0 need more time to
> + * train from Gen1 to Gen2
> + */
> + msleep(100);
> +}
> +
> +static int mtk_pcie_enable_ports(struct mtk_pcie *pcie)
> +{
> + struct device *dev = pcie->dev;
> + struct mtk_pcie_port *port, *tmp;
> + int err, linkup = 0;
> +
> + list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> + err = clk_prepare_enable(port->sys_ck);
> + if (err) {
> + dev_err(dev, "failed to enable port%d clock\n",
> + port->index);
> + continue;
> + }
> +
> + /* assert RC */
> + reset_control_assert(port->reset);
> + /* de-assert RC */
> + reset_control_deassert(port->reset);
> +
> + /* power on PHY */
> + err = phy_power_on(port->phy);
> + if (err) {
> + dev_err(dev, "failed to power on port%d phy\n",
> + port->index);
> + goto err_phy_on;
> + }
> +
> + mtk_pcie_assert_ports(port);
> +
> + /* if link up, then setup root port configuration space */
> + if (mtk_pcie_link_is_up(port)) {
> + mtk_pcie_configure_rc(port);
> + linkup++;
> + continue;
> + }
> +
> + dev_info(dev, "Port%d link down\n", port->index);
> +
> + phy_power_off(port->phy);
> +err_phy_on:
> + clk_disable_unprepare(port->sys_ck);
> + mtk_pcie_port_free(port);
> + }
> +
> + return linkup;
> +}
> +
> +static int mtk_pcie_get_port_resource(struct mtk_pcie_port *port,
> + struct device_node *node)
> +{
> + struct device *dev = port->pcie->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct platform_device *plat_dev;
> + char name[10];
> + int err;
> +
> + err = of_address_to_resource(node, 0, &port->regs);
> + if (err) {
> + dev_err(dev, "failed to parse address: %d\n", err);
> + return err;
> + }
> +
> + port->base = devm_ioremap_resource(dev, &port->regs);
> + if (IS_ERR(port->base)) {
> + dev_err(dev, "failed to map port%d base\n", port->index);
> + return PTR_ERR(port->base);
> + }
> +
> + plat_dev = of_find_device_by_node(node);
> + if (!plat_dev) {
> + plat_dev = of_platform_device_create(
> + node, NULL,
> + platform_bus_type.dev_root);
> + if (!plat_dev)
> + return -EPROBE_DEFER;
> + }
> +
> + port->dev = &plat_dev->dev;
> +
> + port->irq = platform_get_irq(pdev, port->index);
> + if (!port->irq) {
> + dev_err(dev, "failed to get irq\n");
> + return -ENODEV;
> + }
> +
> + port->sys_ck = devm_clk_get(port->dev, "sys_ck");
> + if (IS_ERR(port->sys_ck)) {
> + dev_err(port->dev, "failed to get port%d clock\n", port->index);
> + return PTR_ERR(port->sys_ck);
> + }
> +
> + port->reset = devm_reset_control_get(port->dev, "pcie-reset");
> + if (IS_ERR(port->reset)) {
> + dev_err(port->dev, "failed to get port%d reset control\n",
> + port->index);
> + return PTR_ERR(port->reset);
> + }
> +
> + snprintf(name, sizeof(name), "pcie-phy%d", port->index);
> + port->phy = devm_of_phy_get(port->dev, node, name);
> + if (IS_ERR(port->phy)) {
> + dev_err(port->dev, "failed to get port%d phy\n", port->index);
> + return PTR_ERR(port->phy);
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_parse_and_add_res(struct mtk_pcie *pcie)
> +{
> + struct device *dev = pcie->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct device_node *node = dev->of_node, *child;
> + struct resource_entry *win, *tmp;
> + struct resource *regs;
> + resource_size_t iobase;
> + int err;
> +
> + /* parse shared resources */
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + pcie->base = devm_ioremap_resource(dev, regs);
> + if (IS_ERR(pcie->base)) {
> + dev_err(dev, "failed to get PCIe base\n");
> + return PTR_ERR(pcie->base);
> + }
> +
> + pcie->free_ck = devm_clk_get(dev, "free_ck");
> + if (IS_ERR(pcie->free_ck)) {
> + dev_err(dev, "failed to get free_ck\n");
> + return PTR_ERR(pcie->free_ck);
> + }
> +
> + err = of_pci_get_host_bridge_resources(node, 0, 0xff, &pcie->resources,
> + &iobase);
> + if (err)
> + return err;
> +
> + err = devm_request_pci_bus_resources(dev, &pcie->resources);
> + if (err)
> + return err;
> +
> + resource_list_for_each_entry_safe(win, tmp, &pcie->resources) {
> + struct resource *res = win->res;
> +
> + switch (resource_type(res)) {
> + case IORESOURCE_IO:
> + err = pci_remap_iospace(res, iobase);
> + if (err) {
> + dev_warn(dev, "failed to map resource %pR\n",
> + res);
> + resource_list_destroy_entry(win);
> + }
> + break;
> + }
> + }
> +
> + /* parse port resources */
> + for_each_child_of_node(node, child) {
> + struct mtk_pcie_port *port;
> + int index;
> +
> + err = of_pci_get_devfn(child);
> + if (err < 0) {
> + dev_err(pcie->dev, "failed to parse devfn: %d\n", err);
dev_err(dev, ...)
> + return err;
> + }
> +
> + index = PCI_SLOT(err);
> + if (index < 1) {
> + dev_err(dev, "invalid port number: %d\n", index);
> + return -EINVAL;
> + }
> +
> + index--;
> +
> + if (!of_device_is_available(child))
> + continue;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + err = of_property_read_u32(child, "num-lanes", &port->lane);
> + if (err) {
> + dev_err(dev, "missing num-lanes property\n");
> + return err;
> + }
> +
> + port->index = index;
> + port->pcie = pcie;
> +
> + err = mtk_pcie_get_port_resource(port, child);
> + if (err)
> + return err;
> +
> + INIT_LIST_HEAD(&port->list);
> + list_add_tail(&port->list, &pcie->ports);
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * This IP lacks interrupt status register to check or map INTx from
> + * different devices at the same time.
> + */
> +static int __init mtk_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> +{
> + struct mtk_pcie *pcie = dev->bus->sysdata;
> + struct mtk_pcie_port *port;
> +
> + list_for_each_entry(port, &pcie->ports, list)
> + if (port->index == slot)
> + return port->irq;
> +
> + return -1;
> +}
> +
> +static int mtk_pcie_register_ports(struct mtk_pcie *pcie)
> +{
> + struct pci_bus *bus, *child;
> +
> + bus = pci_scan_root_bus(pcie->dev, 0, &mtk_pcie_ops, pcie,
> + &pcie->resources);
> + if (!bus) {
> + dev_err(pcie->dev, "failed to create root bus\n");
> + return -ENOMEM;
> + }
> +
> + if (!pci_has_flag(PCI_PROBE_ONLY)) {
> + pci_fixup_irqs(pci_common_swizzle, mtk_pcie_map_irq);
> + pci_bus_size_bridges(bus);
> + pci_bus_assign_resources(bus);
> +
> + list_for_each_entry(child, &bus->children, node)
> + pcie_bus_configure_settings(child);
Do you actually need the functionality of PCI_PROBE_ONLY? We're
trying to get rid of this, so if you don't need it, please omit it.
If you *do* need it, can you include a note about why?
If you do need it, I don't think PCI_PROBE_ONLY should control
pci_fixup_irqs() or pcie_bus_configure_settings(). I know there is
some other similar code that does this, but I think PCI_PROBE_ONLY
should only influence resource assignment, i.e., BARs and bridge
windows. I don't want it to influence IRQs or the MPS/MRRS settings
done by pcie_bus_configure_settings() if we can avoid it.
> + }
> +
> + pci_bus_add_devices(bus);
> +
> + return 0;
> +}
> +
> +static int mtk_pcie_probe(struct platform_device *pdev)
> +{
> + struct mtk_pcie *pcie;
> + int err;
> +
> + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pcie->dev = &pdev->dev;
> + platform_set_drvdata(pdev, pcie);
> +
> + /*
> + * parse PCI ranges, configuration bus range and
> + * request their resources
> + */
> + INIT_LIST_HEAD(&pcie->ports);
> + INIT_LIST_HEAD(&pcie->resources);
> +
> + err = mtk_pcie_parse_and_add_res(pcie);
> + if (err)
> + goto err_parse;
> +
> + pm_runtime_enable(pcie->dev);
> + err = pm_runtime_get_sync(pcie->dev);
> + if (err)
> + goto err_pm;
> +
> + err = clk_prepare_enable(pcie->free_ck);
> + if (err) {
> + dev_err(pcie->dev, "failed to enable free_ck\n");
> + goto err_free_ck;
> + }
> +
> + /* power on PCIe ports */
> + err = mtk_pcie_enable_ports(pcie);
> + if (!err)
> + goto err_enable;
> +
> + /* register PCIe ports */
> + err = mtk_pcie_register_ports(pcie);
> + if (err)
> + goto err_enable;
> +
> + return 0;
> +
> +err_enable:
> + clk_disable_unprepare(pcie->free_ck);
> +err_free_ck:
> + pm_runtime_put_sync(pcie->dev);
> +err_pm:
> + pm_runtime_disable(pcie->dev);
> +err_parse:
> + pci_free_resource_list(&pcie->resources);
> +
> + return err;
> +}
> +
> +static const struct of_device_id mtk_pcie_ids[] = {
> + { .compatible = "mediatek,mt7623-pcie"},
> + { .compatible = "mediatek,mt2701-pcie"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_ids);
> +
> +static struct platform_driver mtk_pcie_driver = {
> + .probe = mtk_pcie_probe,
> + .driver = {
> + .name = "mtk-pcie",
> + .of_match_table = mtk_pcie_ids,
Per [1], I think you should have ".suppress_bind_attrs = true," here.
Without it, apparently you can easily crash the system by unbinding
the driver, as in [2].
[1] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=65e0527b933a
[2] https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=073d3dbe9a7c
> + },
> +};
> +
> +builtin_platform_driver(mtk_pcie_driver);
> +
> +MODULE_DESCRIPTION("Mediatek PCIe host driver for MT7623 SoCs families");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: Add the ethernet and ethernet PHY to the cygnus core DT.
From: Florian Fainelli @ 2017-04-24 21:54 UTC (permalink / raw)
To: Eric Anholt, Vivien Didelot, Andrew Lunn,
netdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Ray Jui,
Scott Branden, Jon Mason
In-Reply-To: <20170424215022.30382-3-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
On 04/24/2017 02:50 PM, Eric Anholt wrote:
> Cygnus has a single amac controller connected to the B53 switch with 2
> PHYs. On the BCM911360_EP platform, those two PHYs are connected to
> the external ethernet jacks.
>
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
This looks fine, just a few nits on the label names:
> ---
> arch/arm/boot/dts/bcm-cygnus.dtsi | 60 ++++++++++++++++++++++++++++++++++
> arch/arm/boot/dts/bcm911360_entphn.dts | 8 +++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
> index 009f1346b817..318899df9972 100644
> --- a/arch/arm/boot/dts/bcm-cygnus.dtsi
> +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
> @@ -142,6 +142,56 @@
> interrupts = <0>;
> };
>
> + mdio: mdio@18002000 {
> + compatible = "brcm,iproc-mdio";
> + reg = <0x18002000 0x8>;
> + #size-cells = <1>;
> + #address-cells = <0>;
> +
> + gphy0: eth-gphy@0 {
> + reg = <0>;
> + max-speed = <1000>;
> + };
> +
> + gphy1: eth-gphy@1 {
> + reg = <1>;
> + max-speed = <1000>;
> + };
> + };
> +
> + dsa: dsa@18007000 {
This would be better named switch: switch@18007000
> + compatible = "brcm,bcm11360-srab", "brcm,cygnus-srab";
> + reg = <0x18007000 0x1000>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port0@0 {
You can probably just put port@0
> + reg = <0>;
> + phy-handle = <&gphy0>;
> + phy-mode = "rgmii";
> + };
> +
> + port1@1 {
And so on
> + reg = <1>;
> + phy-handle = <&gphy1>;
> + phy-mode = "rgmii";
> + };
> +
> + port8@8 {
And so forth
> + reg = <8>;
> + label = "cpu";
> + ethernet = <ð0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +
> i2c0: i2c@18008000 {
> compatible = "brcm,cygnus-iproc-i2c", "brcm,iproc-i2c";
> reg = <0x18008000 0x100>;
> @@ -295,6 +345,16 @@
> status = "disabled";
> };
>
> + eth0: enet@18042000 {
> + compatible = "brcm,amac";
> + reg = <0x18042000 0x1000>,
> + <0x18110000 0x1000>;
> + reg-names = "amac_base", "idm_base";
> + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> + max-speed = <1000>;
> + status = "disabled";
> + };
> +
> nand: nand@18046000 {
> compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
> reg = <0x18046000 0x600>, <0xf8105408 0x600>,
> diff --git a/arch/arm/boot/dts/bcm911360_entphn.dts b/arch/arm/boot/dts/bcm911360_entphn.dts
> index 8b3800f46288..2a1f54ab3574 100644
> --- a/arch/arm/boot/dts/bcm911360_entphn.dts
> +++ b/arch/arm/boot/dts/bcm911360_entphn.dts
> @@ -57,6 +57,14 @@
> };
> };
>
> +&dsa {
> + status = "okay";
> +};
And that would be &switch here then.
With that fixed:
Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +
> +ð0 {
> + status = "okay";
> +};
> +
> &uart3 {
> status = "okay";
> };
>
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] net: dsa: b53: Add compatible strings for the Cygnus-family BCM11360.
From: Florian Fainelli @ 2017-04-24 21:51 UTC (permalink / raw)
To: Eric Anholt, Vivien Didelot, Andrew Lunn,
netdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Ray Jui,
Scott Branden, Jon Mason
In-Reply-To: <20170424215022.30382-2-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
On 04/24/2017 02:50 PM, Eric Anholt wrote:
> Cygnus is a small family of SoCs, of which we currently have
> devicetree for BCM11360 and BCM58300. The 11360's B53 is mostly the
> same as 58xx, just requiring a tiny bit of setup that was previously
> missing.
>
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] ARM: dts: Add the ethernet and ethernet PHY to the cygnus core DT.
From: Eric Anholt @ 2017-04-24 21:50 UTC (permalink / raw)
To: Florian Fainelli, Vivien Didelot, Andrew Lunn, netdev,
Rob Herring, Mark Rutland, devicetree
Cc: linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list, Ray Jui,
Scott Branden, Jon Mason, Eric Anholt
In-Reply-To: <20170424215022.30382-1-eric@anholt.net>
Cygnus has a single amac controller connected to the B53 switch with 2
PHYs. On the BCM911360_EP platform, those two PHYs are connected to
the external ethernet jacks.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
arch/arm/boot/dts/bcm-cygnus.dtsi | 60 ++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/bcm911360_entphn.dts | 8 +++++
2 files changed, 68 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 009f1346b817..318899df9972 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -142,6 +142,56 @@
interrupts = <0>;
};
+ mdio: mdio@18002000 {
+ compatible = "brcm,iproc-mdio";
+ reg = <0x18002000 0x8>;
+ #size-cells = <1>;
+ #address-cells = <0>;
+
+ gphy0: eth-gphy@0 {
+ reg = <0>;
+ max-speed = <1000>;
+ };
+
+ gphy1: eth-gphy@1 {
+ reg = <1>;
+ max-speed = <1000>;
+ };
+ };
+
+ dsa: dsa@18007000 {
+ compatible = "brcm,bcm11360-srab", "brcm,cygnus-srab";
+ reg = <0x18007000 0x1000>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port0@0 {
+ reg = <0>;
+ phy-handle = <&gphy0>;
+ phy-mode = "rgmii";
+ };
+
+ port1@1 {
+ reg = <1>;
+ phy-handle = <&gphy1>;
+ phy-mode = "rgmii";
+ };
+
+ port8@8 {
+ reg = <8>;
+ label = "cpu";
+ ethernet = <ð0>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+
i2c0: i2c@18008000 {
compatible = "brcm,cygnus-iproc-i2c", "brcm,iproc-i2c";
reg = <0x18008000 0x100>;
@@ -295,6 +345,16 @@
status = "disabled";
};
+ eth0: enet@18042000 {
+ compatible = "brcm,amac";
+ reg = <0x18042000 0x1000>,
+ <0x18110000 0x1000>;
+ reg-names = "amac_base", "idm_base";
+ interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+ max-speed = <1000>;
+ status = "disabled";
+ };
+
nand: nand@18046000 {
compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
reg = <0x18046000 0x600>, <0xf8105408 0x600>,
diff --git a/arch/arm/boot/dts/bcm911360_entphn.dts b/arch/arm/boot/dts/bcm911360_entphn.dts
index 8b3800f46288..2a1f54ab3574 100644
--- a/arch/arm/boot/dts/bcm911360_entphn.dts
+++ b/arch/arm/boot/dts/bcm911360_entphn.dts
@@ -57,6 +57,14 @@
};
};
+&dsa {
+ status = "okay";
+};
+
+ð0 {
+ status = "okay";
+};
+
&uart3 {
status = "okay";
};
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] net: dsa: b53: Add compatible strings for the Cygnus-family BCM11360.
From: Eric Anholt @ 2017-04-24 21:50 UTC (permalink / raw)
To: Florian Fainelli, Vivien Didelot, Andrew Lunn, netdev,
Rob Herring, Mark Rutland, devicetree
Cc: Scott Branden, Jon Mason, Ray Jui, linux-kernel, Eric Anholt,
bcm-kernel-feedback-list, linux-arm-kernel
In-Reply-To: <20170424215022.30382-1-eric@anholt.net>
Cygnus is a small family of SoCs, of which we currently have
devicetree for BCM11360 and BCM58300. The 11360's B53 is mostly the
same as 58xx, just requiring a tiny bit of setup that was previously
missing.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
Documentation/devicetree/bindings/net/dsa/b53.txt | 3 +++
drivers/net/dsa/b53/b53_srab.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/b53.txt b/Documentation/devicetree/bindings/net/dsa/b53.txt
index d6c6e41648d4..49c93d3c0839 100644
--- a/Documentation/devicetree/bindings/net/dsa/b53.txt
+++ b/Documentation/devicetree/bindings/net/dsa/b53.txt
@@ -29,6 +29,9 @@ Required properties:
"brcm,bcm58625-srab"
"brcm,bcm88312-srab" and the mandatory "brcm,nsp-srab string
+ For the BCM11360 SoC, must be:
+ "brcm,bcm11360-srab" and the mandatory "brcm,cygnus-srab string
+
For the BCM63xx/33xx SoCs with an integrated switch, must be one of:
"brcm,bcm3384-switch"
"brcm,bcm6328-switch"
diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
index 8a62b6a69703..c37ffd1b6833 100644
--- a/drivers/net/dsa/b53/b53_srab.c
+++ b/drivers/net/dsa/b53/b53_srab.c
@@ -364,6 +364,7 @@ static const struct of_device_id b53_srab_of_match[] = {
{ .compatible = "brcm,bcm53018-srab" },
{ .compatible = "brcm,bcm53019-srab" },
{ .compatible = "brcm,bcm5301x-srab" },
+ { .compatible = "brcm,bcm11360-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,bcm58522-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,bcm58525-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,bcm58535-srab", .data = (void *)BCM58XX_DEVICE_ID },
@@ -371,6 +372,7 @@ static const struct of_device_id b53_srab_of_match[] = {
{ .compatible = "brcm,bcm58623-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,bcm58625-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,bcm88312-srab", .data = (void *)BCM58XX_DEVICE_ID },
+ { .compatible = "brcm,cygnus-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ .compatible = "brcm,nsp-srab", .data = (void *)BCM58XX_DEVICE_ID },
{ /* sentinel */ },
};
--
2.11.0
^ permalink raw reply related
* [PATCH 0/2] net: dsa: b53: BCM11360 support
From: Eric Anholt @ 2017-04-24 21:50 UTC (permalink / raw)
To: Florian Fainelli, Vivien Didelot, Andrew Lunn, netdev,
Rob Herring, Mark Rutland, devicetree
Cc: linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list, Ray Jui,
Scott Branden, Jon Mason, Eric Anholt
This little patch series follows on from Florian's fixes that he just
sent, and enables the ethernet on the 911360_EP board.
Without Florian's fixes, the controller comes up and forwarding
happens between eth1 and eth2, but the CPU's eth0 packets don't get
forwarded.
Eric Anholt (2):
net: dsa: b53: Add compatible strings for the Cygnus-family BCM11360.
ARM: dts: Add the ethernet and ethernet PHY to the cygnus core DT.
Documentation/devicetree/bindings/net/dsa/b53.txt | 3 ++
arch/arm/boot/dts/bcm-cygnus.dtsi | 60 +++++++++++++++++++++++
arch/arm/boot/dts/bcm911360_entphn.dts | 8 +++
drivers/net/dsa/b53/b53_srab.c | 2 +
4 files changed, 73 insertions(+)
--
2.11.0
^ permalink raw reply
* Re: [PATCH 1/3] of: overlay_adjust_phandles() - do not modify const field
From: Rob Herring @ 2017-04-24 21:40 UTC (permalink / raw)
To: Frank Rowand, Benjamin Herrenschmidt
Cc: Stephen Boyd, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <58FE49F4.7060600-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Ben H
On Mon, Apr 24, 2017 at 1:54 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 04/24/17 09:56, Rob Herring wrote:
>> On Mon, Apr 24, 2017 at 12:20 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>>
>>> When adjusting overlay phandles to apply to the live device tree, can
>>> not modify the property value because it is type const.
>>>
>>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>>> the type of struct property.value from void * to const void *. As
>>> a result of the type change, the overlay code had compile errors
>>> where the resolver updates phandle values.
>>
>> Conceptually, I prefer your first version. phandles are special and
>> there's little reason to expose them except to generate a dts or dtb
>> from /proc/device-tree. We could still generate the phandle file in
>> that case, but I don't know if special casing phandle is worth it.
>
> The biggest thing that makes me wary about my first version is PPC
> and Sparc. I can read their code, but do not know what the firmware
> is feeding into the kernel, so I felt like there might be some
> incorrect assumptions or fundamental misunderstandings that I
> may have.
I never let that stop me. ;) I guess one question is does
"ibm,phandle" need to be exposed to userspace. Maybe Ben has some
thoughts.
> If we do remove the phandle properties from the live tree, I think
> that phandle still needs to be exposed in /proc/device-tree
> because that is important information for being able to understand
> (or debug) code via reading the source. It isn't a lot code.
>
> One factor I was not sure of to help choose between the first version
> and this approach is net memory size of the device tree:
>
> first version:
>
> Adds struct bin_attribute (28 bytes on 32 bit arm) to every node
You could do a pointer to the struct instead.
> Removes "linux,phandle" and "phandle" properties from nodes that
> have a phandle (64 + 72 = 136 bytes)
I don't think it is that high because we don't copy the property names
and values. It's just 2x sizeof(struct property) plus whatever
overhead each unflatten_dt_alloc has.
> second version plus subsequent "linux,phandle" removal:
>
> Removes "linux,phandle" properties from nodes
> that have a phandle (72 bytes)
>
> I do not have a feel of how many nodes have phandles in the many
> different device trees, so I'm not sure of the end result for the
> first version.
Probably well under half. Though in theory dtc could create a phandle
for every node.
> I do not have a strong preference between my first approach and second
> approach. But now that I have done both, a choice can be made. Let me
> know which way you want to go and I'll respin the one you prefer.
> For this version I'll make the change you suggested. For the first
> version, I'll modify of_attach_mode() slightly more to remove any
> "phandle", "linux,phandle", and "ibm,phandle" property from the node
> before attaching it, and add the call to add the phandle sysfs
> file: __of_add_phandle_sysfs(np);
I still lean toward the former, but I guess it depends if there are
really any size savings.
Why would you remove properties in of_attach_mode? You would want to
convert populate_properties() to store the phandle from the start and
never create the properties.
[...]
>>> + prop = __of_find_property(overlay, "phandle", NULL);
>>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>>> + GFP_KERNEL);
>>> + if (!newprop)
>>> + return -ENOMEM;
>>> + __of_update_property(overlay, newprop, &prop);
>>> +
>>> + prop = __of_find_property(overlay, "linux,phandle", NULL);
>>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>>> + GFP_KERNEL);
>>
>> There is no reason to support "linux,phandle" for overlays. That is
>> legacy (pre ePAPR) which predates any overlays by a long time.
>
> I would like to have the same behavior for non-overlays as for overlays.
> The driver is the same whether the device tree description comes from
> the initial device tree or from an overlay.
Agreed. You only need to store one of them for both base and overlays.
You could go further and just ignore them altogether for overlays as
we should never have any with linux,phandle only, but that doesn't
really matter as it won't affect the memory usage.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] rtc: ds1374: Add trickle charger device tree binding
From: Moritz Fischer @ 2017-04-24 21:28 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Rob Herring, rtc-linux, Devicetree List, Alessandro Zummo,
Mark Rutland
In-Reply-To: <20170424171639.6vv5hxrejylloh3b-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
Hi Alex,
On Mon, Apr 24, 2017 at 07:16:39PM +0200, Alexandre Belloni wrote:
> I think I would do something with MFD as you were suggesting on IRC. You
> can have a look at the flexcom driver (atmel-flexcom.c) which is simply
> selecting a mode and then using whatever IP driver already existed
> (UART, I2C or SPI).
That's an interesting approach, so the idea is to basically use the MFD
to switch an existing driver one way or another via pdata? That could work.
>
> I didn't have a close look but maybe that fits. Then, as you are
> concerned wtih backward compatibility, you could enforce the mode from
> the MFD driver, depending on the CONFIG_RTC_DRV_DS1374_WDT value.
This is where I'm currently at:
https://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux.git/log/?h=wip/mfd-ds1374-rfc
Still a WIP, maybe can get out a patchset by the end of the week ...
Thanks,
Moritz
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [RFC v2 2/2] mux: mmio-based syscon mux controller
From: Peter Rosin @ 2017-04-24 21:05 UTC (permalink / raw)
To: Philipp Zabel
Cc: Rob Herring, Mark Rutland, Sakari Ailus, Steve Longerbeam,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1493050349-25533-2-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 2017-04-24 18:12, Philipp Zabel wrote:
> This adds a driver for mmio-based syscon multiplexers controlled by a
> single bitfield in a syscon register range.
Single bitfield?
>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Changes since v1:
> - Renamed MUX_SYSCON to MUX_MMIO. Instead of obtaining the regmap via syscon,
> this could just as well map its own MMIO register range if a reg property
> was added.
> - Replaced reg, bit-mask, and bit-shift properties with mux-reg-masks array
> to allow defining multiple mux bit-fields per mmio-mux instance.
> - Changed mux-control-cells value to <1>, the cell value is an index into
> the mux-reg-masks array.
> - Replaced idle-state with idle-states array.
> - Stopped clobbering mux->cached_state, that is internal to the mux core.
> ---
> drivers/mux/Kconfig | 13 ++++
> drivers/mux/Makefile | 1 +
> drivers/mux/mux-mmio.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 176 insertions(+)
> create mode 100644 drivers/mux/mux-mmio.c
>
> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
> index 34b8284d6d29e..e2bee25a1586e 100644
> --- a/drivers/mux/Kconfig
> +++ b/drivers/mux/Kconfig
> @@ -43,4 +43,17 @@ config MUX_GPIO
> To compile the driver as a module, choose M here: the module will
> be called mux-gpio.
>
> +config MUX_MMIO
> + tristate "MMIO register bitfield-controlled Multiplexer"
> + depends on OF && MFD_SYSCON
> + help
> + MMIO register bitfield-controlled Multiplexer controller.
> +
> + The driver builds a single multiplexer controller using a bitfield
> + in a syscon register. For N bit wide bitfields, there will be 2^N
> + possible multiplexer states.
Multiple multiplexer controllers....
> +
> + To compile the driver as a module, choose M here: the module will
> + be called mux-mmio.
> +
> endif
> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
> index b00a7d37d2fbe..6bac5b0fea137 100644
> --- a/drivers/mux/Makefile
> +++ b/drivers/mux/Makefile
> @@ -5,3 +5,4 @@
> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
> +obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
> diff --git a/drivers/mux/mux-mmio.c b/drivers/mux/mux-mmio.c
> new file mode 100644
> index 0000000000000..0b011fb59f99b
> --- /dev/null
> +++ b/drivers/mux/mux-mmio.c
> @@ -0,0 +1,162 @@
> +/*
> + * MMIO register bitfield-controlled multiplexer driver
> + *
> + * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mux/driver.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +static int mux_mmio_set(struct mux_control *mux, int state)
> +{
> + struct regmap_field **fields = mux_chip_priv(mux->chip);
> + int index = mux - mux->chip->mux;
mux_control_get_index(mux) does exactly this.
> +
> + return regmap_field_write(fields[index], state);
> +}
> +
> +static const struct mux_control_ops mux_mmio_ops = {
> + .set = mux_mmio_set,
> +};
> +
> +static const struct of_device_id mux_mmio_dt_ids[] = {
> + { .compatible = "mmio-mux", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids);
> +
> +static int mux_mmio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct regmap_field **fields;
> + struct mux_chip *mux_chip;
> + struct regmap *regmap;
> + s32 *idle_states;
> + u32 *reg_masks;
> + int num_fields;
> + int ret;
> + int i;
> +
> + regmap = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(dev, "failed to get syscon regmap: %d\n", ret);
> + return ret;
> + }
> +
> + ret = of_property_count_u32_elems(np, "mux-reg-masks");
> + if (ret == 0 || ret % 2)
> + ret = -EINVAL;
> + if (ret > 0) {
> + num_fields = ret / 2;
> +
> + reg_masks = devm_kmalloc(dev, ret * sizeof(u32), GFP_KERNEL);
> + if (!reg_masks)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, "mux-reg-masks",
> + reg_masks, ret);
> + }
> + if (ret < 0) {
> + dev_err(dev, "mux-reg-masks property missing or invalid: %d\n",
> + ret);
> + return ret;
> + }
> +
> + mux_chip = devm_mux_chip_alloc(dev, num_fields, num_fields *
> + sizeof(*fields));
> + if (IS_ERR(mux_chip))
> + return PTR_ERR(mux_chip);
> +
> + fields = mux_chip_priv(mux_chip);
> +
> + for (i = 0; i < num_fields; i++) {
> + struct mux_control *mux = &mux_chip->mux[i];
> + struct reg_field field;
> + u32 *reg_mask = reg_masks + 2 * i;
> + int bits;
u32 mask;
> +
> + field.reg = reg_mask[0];
> + field.msb = fls(reg_mask[1]) - 1;
> + field.lsb = ffs(reg_mask[1]) - 1;
Perhaps add a sanity check? (untested, but you get the idea...)
mask = (BIT(field.msb + 1) - 1) ^ (BIT(field.lsb) - 1);
if (WARN_ON(reg_mask[1] != mask)) {
/* Catches empty reg_mask[1] as well. */
dev_err(...)
return -EINVAL;
}
> + bits = 1 + field.msb - field.lsb;
> +
> + fields[i] = devm_regmap_field_alloc(&pdev->dev, regmap, field);
> + if (IS_ERR(fields[i])) {
> + ret = PTR_ERR(fields[i]);
> + dev_err(&pdev->dev, "failed to get bit-field %d: %d\n",
> + i, ret);
> + return ret;
> + }
> +
> + mux->states = 1 << bits;
> + }
> +
> + devm_kfree(dev, reg_masks);
> +
> + if (of_find_property(np, "idle-states", NULL)) {
> + ret = of_property_count_u32_elems(np, "idle-states");
> + if (ret == num_fields) {
> + idle_states = devm_kmalloc(dev, ret * sizeof(s32),
> + GFP_KERNEL);
> + if (!idle_states)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, "idle-states",
> + idle_states, ret);
> + } else {
> + idle_states = NULL;
> + if (ret >= 0)
> + ret = -EINVAL;
> + }
> + if (ret < 0) {
> + dev_err(dev, "idle-states property invalid: %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < num_fields; i++) {
Hmm, this is the second loop over num_fields with accesses to a complete
copy of a DT array. Is it not possible to use one loop and set up all
aspects of each mux-control using of_property_read_u32_index calls in that
loop? I have the feeling that will look neater...
> + struct mux_control *mux = &mux_chip->mux[i];
> +
> + if (idle_states[i] != MUX_IDLE_AS_IS) {
> + if (idle_states[i] < 0 ||
> + idle_states[i] >= mux->states) {
> + dev_err(dev, "invalid idle-state %u\n",
> + idle_states[i]);
> + return -EINVAL;
> + }
> +
> + mux->idle_state = idle_states[i];
> + }
> + }
> +
> + devm_kfree(dev, idle_states);
> + }
> +
> + mux_chip->ops = &mux_mmio_ops;
> +
> + return devm_mux_chip_register(dev, mux_chip);
> +}
> +
> +static struct platform_driver mux_mmio_driver = {
> + .driver = {
> + .name = "mmio-mux",
> + .of_match_table = of_match_ptr(mux_mmio_dt_ids),
> + },
> + .probe = mux_mmio_probe,
> +};
> +module_platform_driver(mux_mmio_driver);
> +
> +MODULE_DESCRIPTION("MMIO register bitfield-controlled multiplexer driver");
> +MODULE_AUTHOR("Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2 1/2] dt-bindings: add mmio-based syscon mux controller DT bindings
From: Peter Rosin @ 2017-04-24 21:04 UTC (permalink / raw)
To: Philipp Zabel
Cc: Rob Herring, Mark Rutland, Sakari Ailus, Steve Longerbeam,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1493050349-25533-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 2017-04-24 18:12, Philipp Zabel wrote:
> This adds device tree binding documentation for mmio-based syscon
> multiplexers controlled by a single bitfield in a syscon register
> range.
Single bitfield?
>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Changes since v1:
> - Replaced reg, bit-mask, and bit-shift properties with mux-reg-masks array
> to allow defining multiple mux bit-fields per mmio-mux instance.
> - Changed mux-control-cells value to <1>, the cell value is an index into
> the mux-reg-masks array.
> - Replaced idle-state with idle-states array.
> ---
> Documentation/devicetree/bindings/mux/mmio-mux.txt | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mux/mmio-mux.txt
>
> diff --git a/Documentation/devicetree/bindings/mux/mmio-mux.txt b/Documentation/devicetree/bindings/mux/mmio-mux.txt
> new file mode 100644
> index 0000000000000..99282fa761c55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mux/mmio-mux.txt
> @@ -0,0 +1,60 @@
> +MMIO register bitfield-based multiplexer controller bindings
> +
> +Define register bitfields to be used to control multiplexers. The parent
> +device tree node must be a syscon node to provide register access.
> +
> +Required properties:
> +- compatible : "mmio-mux"
> +- #mux-control-cells : <1>
> +- mux-reg-masks : an array of register offset and pre-shifted bitfield mask
> + pairs, each describing a single mux control.
> +* Standard mux-controller bindings as decribed in mux-controller.txt
> +
> +Optional properties:
> +- idle-states : if present, the state the muxes will have when idle. The
> + special state MUX_IDLE_AS_IS is the default.
> +
> +The multiplexer state is defined as the value of the bitfield described
> +by the reg, bit-mask, and bit-shift properties, accessed through the parent
> +syscon.
This paragraph needs updating.
> +
> +Example:
> +
> + syscon {
> + compatible = "syscon";
> +
> + mux: mux-controller@3 {
You shouldn't do ...@3 if you don't have a reg property.
Cheers,
peda
> + compatible = "mmio-mux";
> + #mux-control-cells = <1>;
> +
> + mux-reg-masks = <0x3 0x30>, /* 0: reg 0x3, bits 5:4 */
> + <0x3 0x40>, /* 1: reg 0x3, bit 6 */
> + idle-states = <MUX_IDLE_AS_IS>, <0>;
> + };
> + };
> +
> + video-mux {
> + compatible = "video-mux";
> + mux-controls = <&mux 0>;
> +
> + ports {
> + /* inputs 0..3 */
> + port@0 {
> + reg = <0>;
> + };
> + port@1 {
> + reg = <1>;
> + };
> + port@2 {
> + reg = <2>;
> + };
> + port@3 {
> + reg = <3>;
> + };
> +
> + /* output */
> + port@4 {
> + reg = <4>;
> + };
> + };
> + };
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching
From: Rafał Miłecki @ 2017-04-24 20:53 UTC (permalink / raw)
To: Jonas Gorski, Rob Herring
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Mark Rutland, Frank Rowand,
Linus Walleij, MTD Maling List,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Geert Uytterhoeven, Rafał Miłecki
In-Reply-To: <CAOiHx=njT+y6VqTbqRFQZ4rJNW6A8XsngTe2WRp=qND9c3ySpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04/24/2017 05:31 PM, Jonas Gorski wrote:
> On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Partition parsers can now provide an of_match_table to enable
>> flash<-->parser matching via device tree.
>>
>> This support is currently limited to built-in parsers as it uses
>> request_module() and friends. This should be sufficient for most cases
>> though as compiling parsers as modules isn't a common choice.
>>
>> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> This is based on Brian's patches:
>> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
>> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>>
>> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>> Merge helpers into a single of_mtd_match_mtd_parser
>> ---
>> drivers/mtd/mtdpart.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>> include/linux/mtd/partitions.h | 1 +
>> 2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 73c52f1a2e4c..d0cb1a892ed2 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>> return ret;
>> }
>>
>> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
>> + struct mtd_part_parser *parser)
>> +{
>> + struct device_node *np;
>> + bool ret;
>> +
>> + np = mtd_get_of_node(mtd);
>> + np = of_get_child_by_name(np, "partitions");
>> +
>> + ret = !!of_match_node(parser->of_match_table, np);
>> +
>> + of_node_put(np);
>> +
>> + return ret;
>> +}
>> +
>> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
>> +{
>> + struct mtd_part_parser *p, *ret = NULL;
>> +
>> + spin_lock(&part_parser_lock);
>> +
>> + list_for_each_entry(p, &part_parsers, list) {
>> + if (of_mtd_match_mtd_parser(mtd, p) &&
>> + try_module_get(p->owner)) {
>> + ret = p;
>> + break;
>> + }
>> + }
>
>
> Hm, maybe iterate over the compatibles, so parsers matching the most
> specific compatible get precedence in case there is more than one
> compatible? Currently it will match the first one that matches any
> compatible, and registration order of parsers can change that. I'm
> thinking of parsers that partially rely on fixed, unprobable layouts,
> so can use "fixed-partitions" as a fallback compatible.
>
> E.g. having something like this
>
> partitions {
> compatible = "sample,custom-layout", "fixed-partitions";
>
> bootloader@0 { ... };
>
> firmware@10000 { .... }; /* will be split by the parser */
>
> extra@780000 { .... }; /* partition the on-flash format can't specify */
> };
>
> Where you will still be able to write an image raw to the image
> partition even if the "custom-layout"-parser isn't present/enabled,
> but if it is present, it should always be used.
I see the point, but I'm afraid we're lacking some DT helper for this. See
below for the function I wrote (and I'm not proud of) - compile tested only.
I think we would need a new helper similar to the of_match_node:
1) Taking const struct of_device_id *matches
2) Taking const struct device_node *node
but returning a score of the best match.
DT guys: any comment on this? Rob?
Would this be acceptable to:
1) Take this patch as is as Linux current doesn't support other bindings
2) Work on DT helper + mtd modification in a separated patchset?
static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
{
struct mtd_part_parser *p, *ret = NULL;
struct device_node *np;
struct property *prop;
const char *cp;
np = mtd_get_of_node(mtd);
np = of_get_child_by_name(np, "partitions");
if (!np)
return NULL;
spin_lock(&part_parser_lock);
of_property_for_each_string(np, "compatible", prop, cp) {
list_for_each_entry(p, &part_parsers, list) {
const struct of_device_id *matches;
for (matches = p->of_match_table;
matches->name[0] || matches->type[0] || matches->compatible[0];
matches++) {
if (!of_compat_cmp(cp, matches->compatible, strlen(matches->compatible)) &&
try_module_get(p->owner)) {
ret = p;
break;
}
}
if (ret)
break;
}
if (ret)
break;
}
spin_unlock(&part_parser_lock);
of_node_put(np);
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] of: Add unit tests for applying overlays.
From: Rob Herring @ 2017-04-24 20:40 UTC (permalink / raw)
To: Frank Rowand
Cc: Stephen Boyd, Michal Marek, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Linux Kbuild mailing list
In-Reply-To: <58FE3A1B.9000503@gmail.com>
On Mon, Apr 24, 2017 at 12:47 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 04/24/17 10:16, Rob Herring wrote:
>> On Mon, Apr 24, 2017 at 12:43 AM, <frowand.list@gmail.com> wrote:
>>> From: Frank Rowand <frank.rowand@sony.com>
>>>
>>> Existing overlay unit tests examine individual pieces of the overlay
>>> code. The new tests target the entire process of applying an overlay.
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
[...]
>>> @@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
>>> */
>>> void __init unflatten_device_tree(void)
>>> {
>>> +#ifdef CONFIG_OF_UNITTEST
>>> + extern uint8_t __dtb_ot_base_begin[];
>>> + extern uint8_t __dtb_ot_base_end[];
>>> + struct device_node *ot_base_root;
>>> + void *ot_base;
>>> + u32 data_size;
>>> + u32 size;
>>> +#endif
>>> +
>>> __unflatten_device_tree(initial_boot_params, NULL, &of_root,
>>> early_init_dt_alloc_memory_arch, false);
>>>
>>> /* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
>>> of_alias_scan(early_init_dt_alloc_memory_arch);
>>
>> Just make __unflatten_device_tree accessible to the unit test code and
>> move all this to it. Then you don't need the ifdefery.
>
> Good idea. I'll do that.
>
>
>> Does this need to be immediately after unflattening the base tree?
>
> My goal is to make the creation of the test data in the tree follow
> the normal process as much as possible, so that real code is tested
> instead of testing test code.
>
> This flattened device tree contains the base information that the
> test overlays are applied against.
Okay. If you need it here, then you can put this all into a unittest
function and call it from here.
>>> +#ifdef CONFIG_OF_OVERLAY
>>> +/*
>>> + * The purpose of of_unittest_overlay_test_data_add is to add an
>>> + * overlay in the normal fashion. This is a test of the whole
>>> + * picture, instead of testing individual elements.
>>> + *
>>> + * A secondary purpose is to be able to verify that the contents of
>>> + * /proc/device-tree/ contains the updated structure and values from
>>> + * the overlay. That must be verified separately in user space.
>>> + *
>>> + * Return 0 on unexpected error.
>>> + */
>>> +static int __init overlay_test_data_add(int onum)
>>
>> There's a need for a general function to apply built-in overlays
>> beyond just unittests. See
>> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c. It's pretty close to the
>> same set of calls.
>
> Yes, agreed.
>
> My plan in the next release cycle is to first clean up drivers/of/overlay.c.
> No functional changes, just cosmetic such as aligning function names with
> what they actually do.
>
> Then make some (hopefully) minor correctness changes, such as locking
> correctly around phandle adjustments.
>
> Then create the general function to apply built-in overlays and convert
> all (two) separate implementations to use the common function. I did
> not want to delay adding the unit tests to wait for this step.
Okay. Whatever order you want to do it is fine.
Rob
^ permalink raw reply
* [PATCH V4] ARM64: dts: hi6220-hikey: Add clock binding for the pmic mfd
From: Daniel Lezcano @ 2017-04-24 20:40 UTC (permalink / raw)
To: xuwei5-C8/M+/jPZTeaMJb+Lgu22Q
Cc: Arnd Bergmann, Stephen Boyd, Michael Turquette, Rob Herring,
Lee Jones, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, open list:OPEN FIRMWARE AND..., open list,
moderated list:ARM/HISILICON SOC...
The hi655x PMIC provides the regulators but also a clock. The latter is missing
in the definition and in the DT, thus it is no possible to enable the WiFi which
depends on this clock.
The hi655x's clock has been added and the hi655x multifunction driver has
updated with a clock-cell.
This patch adds the clock-cells for the PMIC in the DT and updates the
documentation.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
Changelog:
v4:
- Added Acked-by's
- Updated the commit message with a better description
---
Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt | 6 ++++++
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
2 files changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
index 0548569..9630ac0 100644
--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
@@ -16,6 +16,11 @@ Required properties:
- reg: Base address of PMIC on Hi6220 SoC.
- interrupt-controller: Hi655x has internal IRQs (has own IRQ domain).
- pmic-gpios: The GPIO used by PMIC IRQ.
+- #clock-cells: From common clock binding; shall be set to 0
+
+Optional properties:
+- clock-output-names: From common clock binding to override the
+ default output clock name
Example:
pmic: pmic@f8000000 {
@@ -24,4 +29,5 @@ Example:
interrupt-controller;
#interrupt-cells = <2>;
pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+ #clock-cells = <0>;
}
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index dba3c13..e0496f7 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -325,6 +325,7 @@
pmic: pmic@f8000000 {
compatible = "hisilicon,hi655x-pmic";
reg = <0x0 0xf8000000 0x0 0x1000>;
+ #clock-cells = <0>;
interrupt-controller;
#interrupt-cells = <2>;
pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 4/4] ARM: dts: keystone: Add minimum support for K2G ICE evm
From: Franklin S Cooper Jr @ 2017-04-24 20:22 UTC (permalink / raw)
To: robh+dt, linux, ssantosh, devicetree, linux-kernel,
linux-arm-kernel
Cc: Franklin S Cooper Jr
In-Reply-To: <20170424202204.24170-1-fcooper@ti.com>
Add barebones dts support for TI's K2G Industrial Communication Engine evm.
This dts allows the board to boot using a ram based filesystem.
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
arch/arm/boot/dts/Makefile | 3 ++-
arch/arm/boot/dts/keystone-k2g-ice.dts | 35 ++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/keystone-k2g-ice.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..01a98f1 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -193,7 +193,8 @@ dtb-$(CONFIG_ARCH_KEYSTONE) += \
keystone-k2hk-evm.dtb \
keystone-k2l-evm.dtb \
keystone-k2e-evm.dtb \
- keystone-k2g-evm.dtb
+ keystone-k2g-evm.dtb \
+ keystone-k2g-ice.dtb
dtb-$(CONFIG_MACH_KIRKWOOD) += \
kirkwood-b3.dtb \
kirkwood-blackarmor-nas220.dtb \
diff --git a/arch/arm/boot/dts/keystone-k2g-ice.dts b/arch/arm/boot/dts/keystone-k2g-ice.dts
new file mode 100644
index 0000000..d820d37
--- /dev/null
+++ b/arch/arm/boot/dts/keystone-k2g-ice.dts
@@ -0,0 +1,35 @@
+/*
+ * Device Tree Source for K2G Industrial Communication Engine EVM
+ *
+ * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+/dts-v1/;
+
+#include "keystone-k2g.dtsi"
+
+/ {
+ compatible = "ti,k2g-ice", "ti,k2g", "ti,keystone";
+ model = "Texas Instruments K2G Industrial Communication EVM";
+
+ memory@800000000 {
+ device_type = "memory";
+ reg = <0x00000008 0x00000000 0x00000000 0x20000000>;
+ };
+};
+
+&k2g_pinctrl {
+ uart0_pins: pinmux_uart0_pins {
+ pinctrl-single,pins = <
+ K2G_CORE_IOPAD(0x11cc) (BUFFER_CLASS_B | PULL_DISABLE | MUX_MODE0) /* uart0_rxd.uart0_rxd */
+ K2G_CORE_IOPAD(0x11d0) (BUFFER_CLASS_B | PIN_PULLDOWN | MUX_MODE0) /* uart0_txd.uart0_txd */
+ >;
+ };
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pins>;
+ status = "okay";
+};
--
2.10.0
^ permalink raw reply related
* [PATCH v2 3/4] ARM: keystone: Create new binding for K2G ICE evm
From: Franklin S Cooper Jr @ 2017-04-24 20:22 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
ssantosh-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Franklin S Cooper Jr
In-Reply-To: <20170424202204.24170-1-fcooper-l0cyMroinI0@public.gmane.org>
Add a new binding for the new K2G Industrial Communication Engine evm.
Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/arm/keystone/keystone.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/keystone/keystone.txt b/Documentation/devicetree/bindings/arm/keystone/keystone.txt
index 48f6703..f310bad 100644
--- a/Documentation/devicetree/bindings/arm/keystone/keystone.txt
+++ b/Documentation/devicetree/bindings/arm/keystone/keystone.txt
@@ -37,3 +37,6 @@ Boards:
- K2G EVM
compatible = "ti,k2g-evm", "ti,k2g", "ti-keystone"
+
+- K2G Industrial Communication Engine EVM
+ compatible = "ti,k2g-ice", "ti,k2g", "ti-keystone"
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 2/4] ARM: dts: k2g-evm: Add unit address to memory node
From: Franklin S Cooper Jr @ 2017-04-24 20:22 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
ssantosh-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Franklin S Cooper Jr
In-Reply-To: <20170424202204.24170-1-fcooper-l0cyMroinI0@public.gmane.org>
With the new Keystone 2 Industrial Communication EVM adding the
unit address to the memory node it made sense to add it for this board
also.
Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
arch/arm/boot/dts/keystone-k2g-evm.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/keystone-k2g-evm.dts b/arch/arm/boot/dts/keystone-k2g-evm.dts
index 692fcbb..61883cb 100644
--- a/arch/arm/boot/dts/keystone-k2g-evm.dts
+++ b/arch/arm/boot/dts/keystone-k2g-evm.dts
@@ -20,7 +20,7 @@
compatible = "ti,k2g-evm", "ti,k2g", "ti,keystone";
model = "Texas Instruments K2G General Purpose EVM";
- memory {
+ memory@800000000 {
device_type = "memory";
reg = <0x00000008 0x00000000 0x00000000 0x80000000>;
};
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 1/4] ARM: dts: keystone-k2g: Remove skeleton.dtsi
From: Franklin S Cooper Jr @ 2017-04-24 20:22 UTC (permalink / raw)
To: robh+dt, linux, ssantosh, devicetree, linux-kernel,
linux-arm-kernel
Cc: Franklin S Cooper Jr
In-Reply-To: <20170424202204.24170-1-fcooper@ti.com>
Adding the unit address to the memory node was causing the below error:
Warning (reg_format): "reg" property in /memory has invalid length
(8 bytes) (#address-cells == 2, #size-cells == 2)
Further debugging showed that this was due to the memory node added by
default to skeleton.dtsi which was being included in keystone-k2g.dtsi.
Adding a missing node was all that was needed to remove this deprecated
dtsi file from the SoC dtsi. With skeleton.dtsi removed the dtc compiler
no longer complained about including the unit address for the memory node.
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
arch/arm/boot/dts/keystone-k2g.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/keystone-k2g.dtsi b/arch/arm/boot/dts/keystone-k2g.dtsi
index f59567f..a789f75 100644
--- a/arch/arm/boot/dts/keystone-k2g.dtsi
+++ b/arch/arm/boot/dts/keystone-k2g.dtsi
@@ -15,7 +15,6 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/pinctrl/keystone.h>
-#include "skeleton.dtsi"
/ {
compatible = "ti,k2g","ti,keystone";
@@ -24,6 +23,8 @@
#size-cells = <2>;
interrupt-parent = <&gic>;
+ chosen { };
+
aliases {
serial0 = &uart0;
};
--
2.10.0
^ permalink raw reply related
* [PATCH v2 0/4] ARM: dts: keystone: Add support for new K2G evm
From: Franklin S Cooper Jr @ 2017-04-24 20:22 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
ssantosh-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Franklin S Cooper Jr
This patchset adds support for new K2G Industrial Communication Engine
evm. For now only a bare minimal dts which will allow ram boot. Additional
peripherals will be added when base K2G SoC patches are upstreamed allowing
peripherals to be enabled.
Version 2 changes:
Make various tweaks to allow unit address to be added to memory node.
Franklin S Cooper Jr (4):
ARM: dts: keystone-k2g: Remove skeleton.dtsi
ARM: dts: k2g-evm: Add unit address to memory node
ARM: keystone: Create new binding for K2G ICE evm
ARM: dts: keystone: Add minimum support for K2G ICE evm
.../devicetree/bindings/arm/keystone/keystone.txt | 3 ++
arch/arm/boot/dts/Makefile | 3 +-
arch/arm/boot/dts/keystone-k2g-evm.dts | 2 +-
arch/arm/boot/dts/keystone-k2g-ice.dts | 35 ++++++++++++++++++++++
arch/arm/boot/dts/keystone-k2g.dtsi | 3 +-
5 files changed, 43 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/boot/dts/keystone-k2g-ice.dts
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V3 2/2] ARM64: dts: hi6220-hikey: Add clock binding for the pmic mfd
From: Arnd Bergmann @ 2017-04-24 20:19 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Lee Jones, Stephen Boyd, Michael Turquette, Wei Xu,
Linux Kernel Mailing List, linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux ARM, Rob Herring
In-Reply-To: <20170424201139.GF2137@mai>
On Mon, Apr 24, 2017 at 10:11 PM, Daniel Lezcano
<daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Apr 24, 2017 at 09:59:44AM +0100, Lee Jones wrote:
>> On Sat, 22 Apr 2017, Daniel Lezcano wrote:
>>
>> > On 22/04/2017 04:02, Stephen Boyd wrote:
>> > > On 04/17, Daniel Lezcano wrote:
>> > >> Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > >> ---
>> > >> Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt | 6 ++++++
>> > >> arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
>> > >> 2 files changed, 7 insertions(+)
>> > >>
>> > >
>> > > I take it this goes through arm-soc? Not sure why I'm on To:
>> > > line.
>> >
>> > Probably it should go through Lee's tree.
>>
>> Unlikely.
>>
>> The document and the DTS change should really have gone separately,
>> but to save you from having to mess around so close to the merge window:
>>
>> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> So who is supposed to take this patch? Xu? Rob?
The DTS file changes should normally go through the platform maintainer
and from there to arm-soc, to minimize the risk for patch conflicts.
For binding changes, conflicts are much rarer, so we any of the tree
(arm-soc, driver subsystem, or devicetree) works equally well IMHO.
In this particular case,keeping the two together is best, so please send
it to Wei Xu with the added chagnelog and Acks.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.
From: Eric Anholt @ 2017-04-24 20:12 UTC (permalink / raw)
To: dri-devel, Rob Herring, Mark Rutland, devicetree; +Cc: linux-kernel
In-Reply-To: <7906db2f-cfb8-e2e6-5869-b6e829dd8c6f@gmail.com>
For the Raspberry Pi's bindings, the power domain also implicitly
turns on the clock and deasserts reset, but for the new Cygnus port we
start representing the clock in the devicetree.
v2: Document the clock-names property, check for -ENOENT for no clock
in DT.
v3: Drop NULL checks around clk calls which embed NULL checks.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
.../devicetree/bindings/display/brcm,bcm-vc4.txt | 4 +++
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
drivers/gpu/drm/vc4/vc4_v3d.c | 31 +++++++++++++++++++++-
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index ca02d3e4db91..2318266f6481 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -59,6 +59,10 @@ Required properties for V3D:
- interrupts: The interrupt number
See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+Optional properties for V3D:
+- clocks: The clock the unit runs on
+- clock-names: Must be "v3d_clk"
+
Required properties for DSI:
- compatible: Should be "brcm,bcm2835-dsi0" or "brcm,bcm2835-dsi1"
- reg: Physical base address and length of the DSI block's registers
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b0967e2f7e88..92eb7d811bf2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -200,6 +200,7 @@ struct vc4_v3d {
struct vc4_dev *vc4;
struct platform_device *pdev;
void __iomem *regs;
+ struct clk *clk;
};
struct vc4_hvs {
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index a88078d7c9d1..465405586591 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -16,6 +16,7 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include "linux/clk.h"
#include "linux/component.h"
#include "linux/pm_runtime.h"
#include "vc4_drv.h"
@@ -305,6 +306,8 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
drm_gem_object_put_unlocked(&vc4->bin_bo->base.base);
vc4->bin_bo = NULL;
+ clk_disable_unprepare(v3d->clk);
+
return 0;
}
@@ -318,6 +321,10 @@ static int vc4_v3d_runtime_resume(struct device *dev)
if (ret)
return ret;
+ ret = clk_prepare_enable(v3d->clk);
+ if (ret != 0)
+ return ret;
+
vc4_v3d_init_hw(vc4->dev);
vc4_irq_postinstall(vc4->dev);
@@ -348,15 +355,37 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
vc4->v3d = v3d;
v3d->vc4 = vc4;
+ v3d->clk = devm_clk_get(dev, "v3d_clk");
+ if (IS_ERR(v3d->clk)) {
+ int ret = PTR_ERR(v3d->clk);
+
+ if (ret == -ENOENT) {
+ /* bcm2835 didn't have a clock reference in the DT. */
+ ret = 0;
+ v3d->clk = NULL;
+ } else {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get V3D clock: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
return -EINVAL;
}
+ ret = clk_prepare_enable(v3d->clk);
+ if (ret != 0)
+ return ret;
+
ret = vc4_allocate_bin_bo(drm);
- if (ret)
+ if (ret) {
+ clk_disable_unprepare(v3d->clk);
return ret;
+ }
/* Reset the binner overflow address/size at setup, to be sure
* we don't reuse an old one.
--
2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: [PATCH V3 2/2] ARM64: dts: hi6220-hikey: Add clock binding for the pmic mfd
From: Daniel Lezcano @ 2017-04-24 20:11 UTC (permalink / raw)
To: Lee Jones
Cc: Stephen Boyd, mturquette, xuwei5, linux-kernel, linux-clk,
devicetree, linux-arm-kernel, arnd, robh
In-Reply-To: <20170424085944.aa5dsc4g6bwm5rgi@dell>
On Mon, Apr 24, 2017 at 09:59:44AM +0100, Lee Jones wrote:
> On Sat, 22 Apr 2017, Daniel Lezcano wrote:
>
> > On 22/04/2017 04:02, Stephen Boyd wrote:
> > > On 04/17, Daniel Lezcano wrote:
> > >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > >> ---
> > >> Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt | 6 ++++++
> > >> arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
> > >> 2 files changed, 7 insertions(+)
> > >>
> > >
> > > I take it this goes through arm-soc? Not sure why I'm on To:
> > > line.
> >
> > Probably it should go through Lee's tree.
>
> Unlikely.
>
> The document and the DTS change should really have gone separately,
> but to save you from having to mess around so close to the merge window:
>
> Acked-by: Lee Jones <lee.jones@linaro.org>
So who is supposed to take this patch? Xu? Rob?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v4)
From: Eric Anholt @ 2017-04-24 20:01 UTC (permalink / raw)
To: Olof Johansson
Cc: Mark Rutland, devicetree@vger.kernel.org, Florian Fainelli,
Stephen Warren, Stefan Wahren, Lee Jones,
linux-kernel@vger.kernel.org, Rob Herring,
Broadcom Kernel Feedback List, linux-rpi-kernel,
linux-arm-kernel@lists.infradead.org, Gerd Hoffmann
In-Reply-To: <CAOesGMhf-MNh+SPZe2YYefyaPk3qxqhZ5ipr86zzK3r02bE81A@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1589 bytes --]
Olof Johansson <olof@lixom.net> writes:
> Hi,
>
> On Wed, Mar 29, 2017 at 5:26 PM, Eric Anholt <eric@anholt.net> wrote:
>> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
>> now, so it's useful to be able to test that mode on an upstream
>> kernel. It's also been useful for me to use the same board for 32-bit
>> and 64-bit development.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>
>> v1: Gerd's patch that put the ../../../arm64/... link in the Makefile
>> v2: Michael's patch that #included from ../../../arm64/... in a new
>> bcm2837-rpi-3-b.dts.
>> v3: Mine, using symlinks to make sure that we don't break the split DT
>> tree.
>> v4: Rely on the new include/arm64 symlink.
>>
>> Assuming positive review feedback, I assume it would be acceptable to
>> merge the shared/dt-symlinks branch in a PR of my own for the 32-bit
>> DT branch?
>>
>> arch/arm/boot/dts/Makefile | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 011808490fed..27d258cb50f2 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>> bcm2835-rpi-b-plus.dtb \
>> bcm2835-rpi-a-plus.dtb \
>> bcm2836-rpi-2-b.dtb \
>> + include/arm64/broadcom/bcm2837-rpi-3-b.dtb \
>
> Building straight out of (and into) the include dir is a little odd here.
>
> A tiny wrapper *.dtb in this dir, that just includes a shared dts/dtsi
> would be a lot nicer.
OK, just sent a version with a #include.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)
From: Eric Anholt @ 2017-04-24 20:00 UTC (permalink / raw)
To: Lee Jones, Florian Fainelli, Olof Johansson, Rob Herring,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Gerd Hoffmann,
Eric Anholt
Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
now, so it's useful to be able to test that mode on an upstream
kernel. It's also been useful for me to use the same board for 32-bit
and 64-bit development.
Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
2 files changed, 2 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 011808490fed..eded842d9978 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
bcm2835-rpi-b-plus.dtb \
bcm2835-rpi-a-plus.dtb \
bcm2836-rpi-2-b.dtb \
+ bcm2837-rpi-3-b.dtb \
bcm2835-rpi-zero.dtb
dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm4708-asus-rt-ac56u.dtb \
diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
new file mode 100644
index 000000000000..8c8aa4d1e9b3
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
@@ -0,0 +1 @@
+#include "arm64/broadcom/bcm2837-rpi-3.b.dts"
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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