* Re: [PATCH 10/12] of/irq: Make of_msi_map_rid() PCI bus agnostic
From: Rob Herring @ 2020-05-21 23:17 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Marc Zyngier, Linux IOMMU, linux-acpi, devicetree, PCI,
Rafael J. Wysocki, Joerg Roedel, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Robin Murphy, Catalin Marinas, Will Deacon,
Makarand Pawagi, Diana Craciun, Laurentiu Tudor
In-Reply-To: <20200521130008.8266-11-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> There is nothing PCI bus specific in the of_msi_map_rid()
> implementation other than the requester ID tag for the input
> ID space. Rename requester ID to a more generic ID so that
> the translation code can be used by all busses that require
> input/output ID translations.
>
> Leave a wrapper function of_msi_map_rid() in place to keep
> existing PCI code mapping requester ID syntactically unchanged.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/of/irq.c | 28 ++++++++++++++--------------
> include/linux/of_irq.h | 14 ++++++++++++--
> 2 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 48a40326984f..25d17b8a1a1a 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -576,43 +576,43 @@ void __init of_irq_init(const struct of_device_id *matches)
> }
> }
>
> -static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> - u32 rid_in)
> +static u32 __of_msi_map_id(struct device *dev, struct device_node **np,
> + u32 id_in)
> {
> struct device *parent_dev;
> - u32 rid_out = rid_in;
> + u32 id_out = id_in;
>
> /*
> * Walk up the device parent links looking for one with a
> * "msi-map" property.
> */
> for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> - if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> - "msi-map-mask", np, &rid_out))
> + if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
> + "msi-map-mask", np, &id_out))
> break;
> - return rid_out;
> + return id_out;
> }
>
> /**
> - * of_msi_map_rid - Map a MSI requester ID for a device.
> + * of_msi_map_id - Map a MSI ID for a device.
> * @dev: device for which the mapping is to be done.
> * @msi_np: device node of the expected msi controller.
> - * @rid_in: unmapped MSI requester ID for the device.
> + * @id_in: unmapped MSI ID for the device.
> *
> * Walk up the device hierarchy looking for devices with a "msi-map"
> - * property. If found, apply the mapping to @rid_in.
> + * property. If found, apply the mapping to @id_in.
> *
> - * Returns the mapped MSI requester ID.
> + * Returns the mapped MSI ID.
> */
> -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in)
> +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32 id_in)
> {
> - return __of_msi_map_rid(dev, &msi_np, rid_in);
> + return __of_msi_map_id(dev, &msi_np, id_in);
> }
>
> /**
> * of_msi_map_get_device_domain - Use msi-map to find the relevant MSI domain
> * @dev: device for which the mapping is to be done.
> - * @rid: Requester ID for the device.
> + * @id: Device ID.
> * @bus_token: Bus token
> *
> * Walk up the device hierarchy looking for devices with a "msi-map"
> @@ -625,7 +625,7 @@ struct irq_domain *of_msi_map_get_device_domain(struct device *dev, u32 id,
> {
> struct device_node *np = NULL;
>
> - __of_msi_map_rid(dev, &np, id);
> + __of_msi_map_id(dev, &np, id);
> return irq_find_matching_host(np, bus_token);
> }
>
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 7142a3722758..cf9cb1e545ce 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -55,7 +55,12 @@ extern struct irq_domain *of_msi_map_get_device_domain(struct device *dev,
> u32 id,
> u32 bus_token);
> extern void of_msi_configure(struct device *dev, struct device_node *np);
> -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
> +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32 id_in);
> +static inline u32 of_msi_map_rid(struct device *dev,
> + struct device_node *msi_np, u32 rid_in)
> +{
> + return of_msi_map_id(dev, msi_np, rid_in);
> +}
> #else
> static inline int of_irq_count(struct device_node *dev)
> {
> @@ -93,10 +98,15 @@ static inline struct irq_domain *of_msi_map_get_device_domain(struct device *dev
> static inline void of_msi_configure(struct device *dev, struct device_node *np)
> {
> }
> +static inline u32 of_msi_map_id(struct device *dev,
> + struct device_node *msi_np, u32 id_in)
> +{
> + return id_in;
> +}
> static inline u32 of_msi_map_rid(struct device *dev,
> struct device_node *msi_np, u32 rid_in)
Move this out of the ifdef and you only need it declared once.
But again, I think I'd just kill of_msi_map_rid.
> {
> - return rid_in;
> + return of_msi_map_id(dev, msi_np, rid_in);
> }
> #endif
>
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-21 23:10 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Laurentiu Tudor, Linux IOMMU, linux-acpi, devicetree, PCI,
Rafael J. Wysocki, Joerg Roedel, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Robin Murphy, Catalin Marinas, Will Deacon,
Marc Zyngier, Makarand Pawagi, Diana Craciun
In-Reply-To: <20200521130008.8266-10-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> The existing bindings cannot be used to specify the relationship
> between fsl-mc devices and GIC ITSes.
>
> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> msi-map property.
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> ---
> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30 +++++++++++++++++--
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 9134e9bcca56..b0813b2d0493 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit value called an ICID
> the requester.
>
> The generic 'iommus' property is insufficient to describe the relationship
> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> -the set of possible ICIDs under a root DPRC and how they map to
> -an IOMMU.
> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties are used
> +to define the set of possible ICIDs under a root DPRC and how they map to
> +an IOMMU and a GIC ITS respectively.
>
> For generic IOMMU bindings, see
> Documentation/devicetree/bindings/iommu/iommu.txt.
> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> For arm-smmu binding, see:
> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
>
> +For GICv3 and GIC ITS bindings, see:
> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> +
> Required properties:
>
> - compatible
> @@ -119,6 +122,15 @@ Optional properties:
> associated with the listed IOMMU, with the iommu-specifier
> (i - icid-base + iommu-base).
>
> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> + data.
> +
> + The property is an arbitrary number of tuples of
> + (icid-base,iommu,iommu-base,length).
I'm confused because the example has GIC ITS phandle, not an IOMMU.
What is an iommu-base?
> +
> + Any ICID in the interval [icid-base, icid-base + length) is
> + associated with the listed GIC ITS, with the iommu-specifier
> + (i - icid-base + iommu-base).
> Example:
>
> smmu: iommu@5000000 {
> @@ -128,6 +140,16 @@ Example:
> ...
> };
>
> + gic: interrupt-controller@6000000 {
> + compatible = "arm,gic-v3";
> + ...
> + its: gic-its@6020000 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + ...
> + };
> + };
> +
> fsl_mc: fsl-mc@80c000000 {
> compatible = "fsl,qoriq-mc";
> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> @@ -135,6 +157,8 @@ Example:
> msi-parent = <&its>;
> /* define map for ICIDs 23-64 */
> iommu-map = <23 &smmu 23 41>;
> + /* define msi map for ICIDs 23-64 */
> + msi-map = <23 &its 23 41>;
Seeing 23 twice is odd. The numbers to the right of 'its' should be an
ITS number space.
> #address-cells = <3>;
> #size-cells = <1>;
>
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH 07/12] of/device: Add input id to of_dma_configure()
From: Rob Herring @ 2020-05-21 23:02 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Robin Murphy, Joerg Roedel, Laurentiu Tudor, Linux IOMMU,
linux-acpi, devicetree, PCI, Rafael J. Wysocki, Hanjun Guo,
Bjorn Helgaas, Sudeep Holla, Catalin Marinas, Will Deacon,
Marc Zyngier, Makarand Pawagi, Diana Craciun
In-Reply-To: <20200521130008.8266-8-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> Devices sitting on proprietary busses have a device ID space that
> is owned by the respective bus and related firmware bindings. In order
> to let the generic OF layer handle the input translations to
> an IOMMU id, for such busses the current of_dma_configure() interface
> should be extended in order to allow the bus layer to provide the
> device input id parameter - that is retrieved/assigned in bus
> specific code and firmware.
>
> Augment of_dma_configure() to add an optional input_id parameter,
> leaving current functionality unchanged.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 4 ++-
> drivers/iommu/of_iommu.c | 53 +++++++++++++++++++++------------
> drivers/of/device.c | 8 +++--
> include/linux/of_device.h | 16 ++++++++--
> include/linux/of_iommu.h | 6 ++--
> 5 files changed, 60 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 40526da5c6a6..8ead3f0238f2 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -118,11 +118,13 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
> static int fsl_mc_dma_configure(struct device *dev)
> {
> struct device *dma_dev = dev;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + u32 input_id = mc_dev->icid;
>
> while (dev_is_fsl_mc(dma_dev))
> dma_dev = dma_dev->parent;
>
> - return of_dma_configure(dev, dma_dev->of_node, 0);
> + return of_dma_configure_id(dev, dma_dev->of_node, 0, &input_id);
> }
>
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index ad96b87137d6..4516d5bf6cc9 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -139,25 +139,53 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> return err;
> }
>
> -static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> - struct device_node *master_np)
> +static int of_iommu_configure_dev_id(struct device_node *master_np,
> + struct device *dev,
> + const u32 *id)
Should have read this patch before #6. I guess you could still make
of_pci_iommu_init() call
of_iommu_configure_dev_id.
Rob
^ permalink raw reply
* Re: [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
From: Rob Herring @ 2020-05-21 22:47 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Joerg Roedel, Robin Murphy, Marc Zyngier, Linux IOMMU, linux-acpi,
devicetree, PCI, Rafael J. Wysocki, Hanjun Guo, Bjorn Helgaas,
Sudeep Holla, Catalin Marinas, Will Deacon, Makarand Pawagi,
Diana Craciun, Laurentiu Tudor
In-Reply-To: <20200521130008.8266-7-lorenzo.pieralisi@arm.com>
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> There is nothing PCI specific (other than the RID - requester ID)
> in the of_map_rid() implementation, so the same function can be
> reused for input/output IDs mapping for other busses just as well.
>
> Rename the RID instances/names to a generic "id" tag and provide
> an of_map_rid() wrapper function so that we can leave the existing
> (and legitimate) callers unchanged.
It's not all that clear to a casual observer that RID is a PCI thing,
so I don't know that keeping it buys much. And there's only 3 callers.
> No functionality change intended.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/iommu/of_iommu.c | 2 +-
> drivers/of/base.c | 42 ++++++++++++++++++++--------------------
> include/linux/of.h | 17 +++++++++++++++-
> 3 files changed, 38 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 20738aacac89..ad96b87137d6 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> int err;
>
> - err = of_map_rid(master_np, mc_dev->icid, "iommu-map",
> + err = of_map_id(master_np, mc_dev->icid, "iommu-map",
I'm not sure this is an improvement because I'd refactor this function
and of_pci_iommu_init() into a single function:
of_bus_iommu_init(struct device *dev, struct device_node *np, u32 id)
Then of_pci_iommu_init() becomes:
of_pci_iommu_init()
{
return of_bus_iommu_init(info->dev, info->np, alias);
}
And replace of_fsl_mc_iommu_init call with:
err = of_bus_iommu_init(dev, master_np, to_fsl_mc_device(dev)->icid);
> "iommu-map-mask", &iommu_spec.np,
> iommu_spec.args);
> if (err)
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ae03b1218b06..e000e17bd602 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu)
> }
>
> /**
> - * of_map_rid - Translate a requester ID through a downstream mapping.
> + * of_map_id - Translate a requester ID through a downstream mapping.
Still a requester ID?
> * @np: root complex device node.
> - * @rid: device requester ID to map.
> + * @id: device ID to map.
> * @map_name: property name of the map to use.
> * @map_mask_name: optional property name of the mask to use.
> * @target: optional pointer to a target device node.
> * @id_out: optional pointer to receive the translated ID.
> *
> - * Given a device requester ID, look up the appropriate implementation-defined
> + * Given a device ID, look up the appropriate implementation-defined
> * platform ID and/or the target device which receives transactions on that
> * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> * @id_out may be NULL if only the other is required. If @target points to
> @@ -2219,11 +2219,11 @@ int of_find_last_cache_level(unsigned int cpu)
> *
> * Return: 0 on success or a standard error code on failure.
> */
> -int of_map_rid(struct device_node *np, u32 rid,
> +int of_map_id(struct device_node *np, u32 id,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out)
> {
> - u32 map_mask, masked_rid;
> + u32 map_mask, masked_id;
> int map_len;
> const __be32 *map = NULL;
>
> @@ -2235,7 +2235,7 @@ int of_map_rid(struct device_node *np, u32 rid,
> if (target)
> return -ENODEV;
> /* Otherwise, no map implies no translation */
> - *id_out = rid;
> + *id_out = id;
> return 0;
> }
>
> @@ -2255,22 +2255,22 @@ int of_map_rid(struct device_node *np, u32 rid,
> if (map_mask_name)
> of_property_read_u32(np, map_mask_name, &map_mask);
>
> - masked_rid = map_mask & rid;
> + masked_id = map_mask & id;
> for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> struct device_node *phandle_node;
> - u32 rid_base = be32_to_cpup(map + 0);
> + u32 id_base = be32_to_cpup(map + 0);
> u32 phandle = be32_to_cpup(map + 1);
> u32 out_base = be32_to_cpup(map + 2);
> - u32 rid_len = be32_to_cpup(map + 3);
> + u32 id_len = be32_to_cpup(map + 3);
>
> - if (rid_base & ~map_mask) {
> - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> + if (id_base & ~map_mask) {
> + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
> np, map_name, map_name,
> - map_mask, rid_base);
> + map_mask, id_base);
> return -EFAULT;
> }
>
> - if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> + if (masked_id < id_base || masked_id >= id_base + id_len)
> continue;
>
> phandle_node = of_find_node_by_phandle(phandle);
> @@ -2288,20 +2288,20 @@ int of_map_rid(struct device_node *np, u32 rid,
> }
>
> if (id_out)
> - *id_out = masked_rid - rid_base + out_base;
> + *id_out = masked_id - id_base + out_base;
>
> - pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> - np, map_name, map_mask, rid_base, out_base,
> - rid_len, rid, masked_rid - rid_base + out_base);
> + pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
> + np, map_name, map_mask, id_base, out_base,
> + id_len, id, masked_id - id_base + out_base);
> return 0;
> }
>
> - pr_info("%pOF: no %s translation for rid 0x%x on %pOF\n", np, map_name,
> - rid, target && *target ? *target : NULL);
> + pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
> + id, target && *target ? *target : NULL);
>
> /* Bypasses translation */
> if (id_out)
> - *id_out = rid;
> + *id_out = id;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(of_map_rid);
> +EXPORT_SYMBOL_GPL(of_map_id);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index c669c0a4732f..b7934566a1aa 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -554,10 +554,18 @@ bool of_console_check(struct device_node *dn, char *name, int index);
>
> extern int of_cpu_node_to_id(struct device_node *np);
>
> -int of_map_rid(struct device_node *np, u32 rid,
> +int of_map_id(struct device_node *np, u32 id,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out);
>
> +static inline int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name,
> + const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return of_map_id(np, rid, map_name, map_mask_name, target, id_out);
> +}
> +
> #else /* CONFIG_OF */
>
> static inline void of_core_init(void)
> @@ -978,6 +986,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
> return -ENODEV;
> }
>
> +static inline int of_map_id(struct device_node *np, u32 id,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return -EINVAL;
> +}
> +
> static inline int of_map_rid(struct device_node *np, u32 rid,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out)
> --
> 2.26.1
>
^ permalink raw reply
* Re: [PATCH v2 0/4] TI K3 DSP remoteproc driver for C66x DSPs
From: Mathieu Poirier @ 2020-05-21 22:23 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Suman Anna, Rob Herring, Lokesh Vutla, linux-remoteproc,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20200521190141.GN408178@builder.lan>
Gents,
On Thu, May 21, 2020 at 12:01:41PM -0700, Bjorn Andersson wrote:
> On Thu 21 May 11:59 PDT 2020, Suman Anna wrote:
>
> > Hi Bjorn,
> >
> > On 5/20/20 7:10 PM, Suman Anna wrote:
> > > Hi All,
> > >
> > > The following is v2 of the K3 DSP remoteproc driver supporting the C66x DSPs
> > > on the TI K3 J721E SoCs. The patches are based on the latest commit on the
> > > rproc-next branch, 7dcef3988eed ("remoteproc: Fix an error code in
> > > devm_rproc_alloc()").
> >
> > I realized I also had the R5F patches on my branch, so the third patch won't
> > apply cleanly (conflict on Makefile). Let me know if you want a new revision
> > posted for you to pick up the series.
> >
>
> That should be fine, thanks for the heads up!
>
> Will give Mathieu a day or two to take a look as well.
I don't see having the time to review this set before the middle/end of next
week. I also understand we are crunched by time if we want to get this in
for the upcoming merge window.
If memory serves me well there wasn't anything controversial about this work.
Under normal circumstances I'd give it a final look but I trust Suman to have
carried out what we agreed on.
Bjorn - if you are happy with this set then go ahead and queue it.
Thanks,
Mathieu
>
> Regards,
> Bjorn
>
> > regards
> > Suman
> >
> > >
> > > v2 includes a new remoteproc core patch (patch 1) that adds an OF helper
> > > for parsing the firmware-name property. This is refactored out to avoid
> > > replicating the code in various remoteproc drivers. Please see the
> > > individual patches for detailed changes.
> > >
> > > The main dependent patches from the previous series are now staged in
> > > rproc-next branch. The only dependency for this series is the common
> > > ti-sci-proc helper between R5 and DSP drivers [1]. Please see the initial
> > > cover-letter [2] for v1 details.
> > >
> > > regards
> > > Suman
> > >
> > > [1] https://patchwork.kernel.org/patch/11456379/
> > > [2] https://patchwork.kernel.org/cover/11458573/
> > >
> > > Suman Anna (4):
> > > remoteproc: Introduce rproc_of_parse_firmware() helper
> > > dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs
> > > remoteproc/k3-dsp: Add a remoteproc driver of K3 C66x DSPs
> > > remoteproc/k3-dsp: Add support for L2RAM loading on C66x DSPs
> > >
> > > .../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 190 +++++
> > > drivers/remoteproc/Kconfig | 13 +
> > > drivers/remoteproc/Makefile | 1 +
> > > drivers/remoteproc/remoteproc_core.c | 23 +
> > > drivers/remoteproc/remoteproc_internal.h | 2 +
> > > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 773 ++++++++++++++++++
> > > 6 files changed, 1002 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> > > create mode 100644 drivers/remoteproc/ti_k3_dsp_remoteproc.c
> > >
> >
^ permalink raw reply
* Re: [PATCH v4 03/14] PCI: cadence: Add support to use custom read and write accessors
From: Rob Herring @ 2020-05-21 22:17 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Arnd Bergmann, Tom Joseph,
Greg Kroah-Hartman, PCI, devicetree, linux-kernel@vger.kernel.org,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <37d2d6c3-232d-adb8-4e0b-e0c699ec435a@ti.com>
On Thu, May 21, 2020 at 7:33 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/21/2020 3:37 AM, Rob Herring wrote:
> > On Wed, May 06, 2020 at 08:44:18PM +0530, Kishon Vijay Abraham I wrote:
> >> Add support to use custom read and write accessors. Platforms that
> >> don't support half word or byte access or any other constraint
> >> while accessing registers can use this feature to populate custom
> >> read and write accessors. These custom accessors are used for both
> >> standard register access and configuration space register access of
> >> the PCIe host bridge.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >> drivers/pci/controller/cadence/pcie-cadence.h | 107 +++++++++++++++---
> >> 1 file changed, 94 insertions(+), 13 deletions(-)
> >
> > Actually, take back my R-by...
> >
> >>
> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> >> index df14ad002fe9..70b6b25153e8 100644
> >> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> >> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> >> @@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing {
> >> MSG_ROUTING_GATHER,
> >> };
> >>
> >> +struct cdns_pcie_ops {
> >> + u32 (*read)(void __iomem *addr, int size);
> >> + void (*write)(void __iomem *addr, int size, u32 value);
> >> +};
> >> +
> >> /**
> >> * struct cdns_pcie - private data for Cadence PCIe controller drivers
> >> * @reg_base: IO mapped register base
> >> @@ -239,7 +244,7 @@ struct cdns_pcie {
> >> int phy_count;
> >> struct phy **phy;
> >> struct device_link **link;
> >> - const struct cdns_pcie_common_ops *ops;
> >> + const struct cdns_pcie_ops *ops;
> >> };
> >>
> >> /**
> >> @@ -299,69 +304,145 @@ struct cdns_pcie_ep {
> >> /* Register access */
> >> static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
> >> {
> >> - writeb(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x1, value);
> >> + return;
> >> + }
> >> +
> >> + writeb(value, addr);
> >> }
> >>
> >> static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
> >> {
> >> - writew(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x2, value);
> >> + return;
> >> + }
> >> +
> >> + writew(value, addr);
> >> }
> >
> > cdns_pcie_writeb and cdns_pcie_writew are used, so remove them.
> >
> >>
> >> static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
> >> {
> >> - writel(value, pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x4, value);
> >> + return;
> >> + }
> >> +
> >> + writel(value, addr);
> >
> > writel isn't broken for you, so you don't need this either.
> >
> >> }
> >>
> >> static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
> >> {
> >> - return readl(pcie->reg_base + reg);
> >> + void __iomem *addr = pcie->reg_base + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->read)
> >> + return pcie->ops->read(addr, 0x4);
> >> +
> >> + return readl(addr);
> >
> > And neither is readl.
>
> Sure, I'll remove all the unused functions and avoid using ops for readl and
> writel.
> >
> >> }
> >>
> >> /* Root Port register access */
> >> static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
> >> u32 reg, u8 value)
> >> {
> >> - writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> >> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x1, value);
> >> + return;
> >> + }
> >> +
> >> + writeb(value, addr);
> >> }
> >>
> >> static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
> >> u32 reg, u16 value)
> >> {
> >> - writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> >> + void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
> >> +
> >> + if (pcie->ops && pcie->ops->write) {
> >> + pcie->ops->write(addr, 0x2, value);
> >> + return;
> >> + }
> >> +
> >> + writew(value, addr);
> >
> > You removed 2 out of 3 calls to this. I think I'd just make the root
> > port writes always be 32-bit. It is all just one time init stuff
> > anyways.
> >
> > Either rework the calls to assemble the data into 32-bits or keep these
> > functions and do the RMW here.
>
> The problem with assembling data into 32-bits is we have to read/write with
> different offsets. We'll give PCI_VENDOR_ID offset for modifying deviceID,
> PCI_INTERRUPT_LINE for modifying INTERRUPT_PIN which might get non-intuitive.
> Similarly in endpoint we read and write to MSI_FLAGS (which is at offset 2) we
> have to directly use MSI capability offset.
>
> And doing RMW in the accessors would mean the same RMW op is repeated. So if we
> just have cdns_pcie_rp_writeb() and cdns_pcie_rp_writew(), the same code will
> be repeated here twice.
Why repeated? Just copy what the config accessors do:
static inline void cdns_pcie_write_sz(u32 val, void __iomem *addr, int size)
{
u32 tmp, mask, where = (unsigned long)addr & 0x3;
addr -= where;
mask = ~(((1 << (size * 8)) - 1) << (where * 8));
tmp = readl(addr) & mask;
tmp |= val << (where * 8);
writel(tmp, addr);
}
/* Root Port register access */
static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
u32 reg, u8 value)
{
cdns_pcie_write_sz(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg, 1);
}
static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
u32 reg, u16 value)
{
cdns_pcie_write_sz(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg, 2);
}
>
> IMHO using ops is a lot cleaner for these cases. IMHO except for removing
> unused functions and not changing readl/writel, others should use ops.
Trying to read the DW PCI driver I don't agree...
Again, unless doing RMW is fundamentally broken (like it is for config
space at runtime), then you only want to do it on broken h/w and ops
would be appropriate. It is all mostly one time setup, so doing a few
extra reads isn't a big deal. If you really care about speed on that,
probably should use the _relaxed accessor variants.
I'm being hopeful the Cadence IP doesn't become the mess that DW is.
Rob
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: spi: Add Baikal-T1 System Boot SPI Controller binding
From: Rob Herring @ 2020-05-21 21:35 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Ramil Zaripov, Alexey Malahov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, John Garry,
Chuanhong Guo, Tomer Maimon, Lee Jones, Miquel Raynal,
Arnd Bergmann, open list:MIPS, linux-spi, devicetree,
linux-kernel@vger.kernel.org
In-Reply-To: <20200521151158.f3izg2svbn5dh6hy@mobilestation>
On Thu, May 21, 2020 at 9:12 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Thu, May 21, 2020 at 08:57:10AM -0600, Rob Herring wrote:
> > On Mon, May 18, 2020 at 3:27 PM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > > > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > > > responsible for the system bootup from an external SPI flash. It's a DW
> > > > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > > > native chip-select available and a single reference clock. Since Baikal-T1
> > > > > SoC is normally booted up from an external SPI flash this SPI controller
> > > > > in most of the cases is supposed to be connected to a single SPI-nor
> > > > > flash. Additionally in order to provide a transparent from CPU point of
> > > > > view initial code execution procedure the system designers created an IP
> > > > > block which physically maps the SPI flash found at CS0 to a memory region.
> > > > >
> > > > > Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > > Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > > > Cc: Paul Burton <paulburton@kernel.org>
> > > > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > > > Cc: John Garry <john.garry@huawei.com>
> > > > > Cc: Chuanhong Guo <gch981213@gmail.com>
> > > > > Cc: Tomer Maimon <tmaimon77@gmail.com>
> > > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > > > Cc: linux-mips@vger.kernel.org
> > > > > Cc: linux-spi@vger.kernel.org
> > > > > ---
> > > > > .../bindings/spi/baikal,bt1-sys-ssi.yaml | 100 ++++++++++++++++++
> > > > > 1 file changed, 100 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..d9d3257d78f4
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > > @@ -0,0 +1,100 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Baikal-T1 System Boot SSI Controller
> > > > > +
> > > > > +description: |
> > > > > + Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > > > + responsible for loading chip bootup code from an external SPI flash. In order
> > > > > + to do this transparently from CPU point of view there is a dedicated IP block
> > > > > + mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > > > + the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > > > + no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > > > + (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > > > + just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > > > + exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > > > + memory. So either one or another way of using the controller functionality
> > > > > + can be enabled at a time.
> > > > > +
> > > > > +maintainers:
> > > > > + - Serge Semin <fancer.lancer@gmail.com>
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: spi-controller.yaml#
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: baikal,bt1-sys-ssi
> > > > > +
> > > > > + reg:
> > > > > + items:
> > > > > + - description: Baikal-T1 Boot Controller configuration registers
> > > > > + - description: Physically mapped SPI flash ROM found at CS0
> > > > > +
> > > > > + reg-names:
> > > > > + items:
> > > > > + - const: config
> > > > > + - const: map
> > > > > +
> > > > > + clocks:
> > > > > + description: SPI Controller reference clock source
> > > >
> > > > Can drop this.
> > >
> > > Ok.
> > >
> > > >
> > > > > + maxItems: 1
> > > > > +
> > > > > + clock-names:
> > > > > + items:
> > > > > + - const: ssi_clk
> > > > > +
> > > > > + num-cs:
> > > > > + const: 1
> > > > > +
> > > > > +patternProperties:
> > > > > + "^.*@[0-9a-f]+":
> > > > > + type: object
> > > > > + properties:
> > > > > + reg:
> > > > > + minimum: 0
> > > > > + maximum: 0
> > > > > +
> > > > > + spi-rx-bus-width:
> > > > > + const: 1
> > > > > +
> > > > > + spi-tx-bus-width:
> > > > > + const: 1
> > > >
> > > > What's the point of these 2 properties if they aren't required?
> > >
> > > Yes, they are optional, but this is a constraint on the bus-width parameters.
> > > DW APB SSI provides a single laned Tx and Rx.
> >
> > Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
> > for example?
>
> Right.
>
> >
> > You could also say 'spi-tx-bus-width: false' here to disallow the
> > property. I guess the above is fine.
>
> Ok. If it's fine I'll leave them as is then. Right?
Yes.
> > > > +unevaluatedProperties: false
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > + - reg-names
> > >
> > > > + - "#address-cells"
> > > > + - "#size-cells"
> > >
> > > These 2 are required by spi-controller.yaml, so you can drop here.
> >
> > Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
> > be like that?
>
> As far as I can see in spi-controller.yaml, "#address-cells" is required, but
> "#size-cells" isn't. Is this intentional?
Humm, I guess we didn't have either one because you can have no child
node defined, but then added #address-cells for the spi-slave case.
In any case, it's all pretty well covered already because the core
schema checks that both are present and dtc has some checking too.
Rob
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-21 21:28 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <9ecb5790-47fe-583b-6fc3-8f4f3ce7860e@linaro.org>
Hi,
On Thu, May 21, 2020 at 8:56 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
> On 21/05/2020 16:10, Doug Anderson wrote:
> >> On 20/05/2020 23:48, Doug Anderson wrote:
> >>>> Is this only applicable for corrected address space?
> >>> I guess I was proposing a two dts-node / two drive approach here.
> >>>
> >>> dts node #1:just covers the memory range for accessing the FEC-corrected data
> >>> driver #1: read-only and reads the FEC-corrected data
> >>>
> >>> dts node #2: covers the memory range that's_not_ the FEC-corrected
> >>> memory range.
> >>> driver #2: read-write. reading reads uncorrected data
> >>>
> >>> Does that seem sane?
> >> I see your point but it does not make sense to have two node for same thing.
> > OK, so that sounds as if we want to go with the proposal where we
> > "deprecate the old driver and/or bindings and say that there really
> > should just be one node and one driver".
> >
> > Would this be acceptable to you?
> >
> > 1. Officially mark the old bindings as deprecated.
>
> Possibly Yes for some reasons below!
>
> >
> > 2. Leave the old driver there to support the old deprecated bindings,
> > at least until everyone can be transferred over. There seem to be
> > quite a few existing users of "qcom,qfprom" and we're supposed to make
> > an attempt at keeping the old device trees working, at least for a
> > little while. Once everyone is transferred over we could decide to
> > delete the old driver.
> we could consider "qcom,qfrom" to be only passing corrected address
> space. Till we transition users to new bindings!
>
> >
> Yes.
>
> > 3. We will have a totally new driver here.
> No, we should still be using the same driver. But the exiting driver
> seems to incorrect and is need of fixing.
>
> Having a look at the memory map for old SoCs like msm8996 and msm8916
> shows that memory map that was passed to qfprom driver is corrected
> address space. Writes will not obviously work!
>
> This should also be true with sdm845 or sc7180
>
> That needs to be fixed first!
OK, so to summarize:
1. We will have one driver: "drivers/nvmem/qfprom.c"
2. If the driver detects that its reg is pointing to the corrected
address space then it should operate in read-only mode. Maybe it can
do this based on the compatible string being just "qcom,qfprom" or
maybe it can do this based on the size of the "reg".
3. If that driver sees a newer compatible string (one that includes
the SoC name in it) it will assume that its "reg" points to the start
of qfprom space.
4. We should post patches to transition all old dts files away from
the deprecated bindings.
> > 4. A given device tree will_not_ be allowed to have both
> > "qcom,qfprom" specified and "qcom,SOC-qfprom" specified. ...and by
> > "qcom,SOC-qfprom" I mean that SOC should be replaced by the SoC name,
> > so "qcom,sc7180-qfprom" or "qcom,sdm845-qfprom". So once you switch
> > to the new node it replaces the old node.
>
> Secondly, this IP is clearly an integral part of Secure Control Block,
> which clearly has versioning information.
>
> Versioning information should be part of compatible string in msm8996 it
> should be "qcom,qfprom-5.1.0"
> for msm8916 it should be "qcom,qfprom-4.0.0" this translates to
> "qcom,qfprom-<MAJOR-NUMBER>-<MINOR-NUMBER>-<STEP>"
I don't know much about this versioning info, but I'm curious: can we
read it from the chip? If so then it actually _doesn't_ need to be in
the compatible string, I think. Device tree shouldn't include things
that can be probed. So if this can be probed then maybe we could have
the compatible as:
compatible = "qcom,msm8996-qfprom", "qcom,qfprom"
...where the SoC is there just in case we need it but we'd expect to
just match on "qcom,qfprom" and then probe.
If this can't be probed then having the version info is nice, so then
I guess you'd have the compatible string:
compatible = "qcom,msm8996-qfprom", "qcom,qfprom-5.1.0"
...where (again) you'd have the SoC specific string there just in case
but you'd expect that you could just use the generic string.
Does that sound right?
> Thirdly we should be able to have common read for all these as they tend
> to just read from corrected address space.
>
> Offsets to corrected address space seems to always constant across SoCs too.
>
> platform specific device tree nodes should also be able to specify
> "read-only" property to not allow writes on to this raw area.
Yeah, I was thinking we probably wanted a read-only property. That
sounds sane to me.
-Doug
^ permalink raw reply
* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Wolfram Sang @ 2020-05-21 21:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Tali Perry, ofery, brendanhiggins, avifishman70, tmaimon77,
kfting, venture, yuenn, benjaminfair, robh+dt, linux-arm-kernel,
linux-i2c, openbmc, devicetree, linux-kernel
In-Reply-To: <20200521143100.GA16812@ninjato>
[-- Attachment #1: Type: text/plain, Size: 192 bytes --]
> From a glimpse, this looks good to go. I will have a close look later
> today.
Phew, this driver is huge. I won't finish my review today, but I am
working on it and am maybe 2/3 through.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v5 6/8] clocksource: dw_apb_timer_of: Fix missing clockevent timers
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano, Rob Herring,
Heiko Stuebner, Dinh Nguyen, Jamie Iles, Arnd Bergmann
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Rob Herring,
linux-mips, linux-rtc, devicetree, Enrico Weigelt,
Greg Kroah-Hartman, Allison Randal, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Commit 100214889973 ("clocksource: dw_apb_timer_of: use
clocksource_of_init") replaced a publicly available driver
initialization method with one called by the timer_probe() method
available after CLKSRC_OF. In current implementation it traverses
all the timers available in the system and calls their initialization
methods if corresponding devices were either in dtb or in acpi. But
if before the commit any number of available timers would be installed
as clockevent and clocksource devices, after that there would be at most
two. The rest are just ignored since default case branch doesn't do
anything. I don't see a reason of such behaviour, neither the commit
message explains it. Moreover this might be wrong if on some platforms
these timers might be used for different purpose, as virtually CPU-local
clockevent timers and as an independent broadcast timer. So in order
to keep the compatibility with the platforms where the order of the
timers detection has some meaning, lets add the secondly discovered
timer to be of clocksource/sched_clock type, while the very first and
the others would provide the clockevents service.
Fixes: 100214889973 ("clocksource: dw_apb_timer_of: use clocksource_of_init")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
drivers/clocksource/dw_apb_timer_of.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 2db490f35c20..ab3ddebe8344 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -147,10 +147,6 @@ static int num_called;
static int __init dw_apb_timer_init(struct device_node *timer)
{
switch (num_called) {
- case 0:
- pr_debug("%s: found clockevent timer\n", __func__);
- add_clockevent(timer);
- break;
case 1:
pr_debug("%s: found clocksource timer\n", __func__);
add_clocksource(timer);
@@ -161,6 +157,8 @@ static int __init dw_apb_timer_init(struct device_node *timer)
#endif
break;
default:
+ pr_debug("%s: found clockevent timer\n", __func__);
+ add_clockevent(timer);
break;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v5 7/8] clocksource: mips-gic-timer: Register as sched_clock
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Paul Burton, Alexey Malahov,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Vincenzo Frascino,
linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
From: Paul Burton <paulburton@kernel.org>
The MIPS GIC timer is well suited for use as sched_clock, so register it
as such.
Whilst the existing gic_read_count() function matches the prototype
needed by sched_clock_register() already, we split it into 2 functions
in order to remove the need to evaluate the mips_cm_is64 condition
within each call since sched_clock should be as fast as possible.
Note the sched clock framework needs the clock source being stable in
order to rely on it. So we register the MIPS GIC timer as schedule clocks
only if it's, if either the system doesn't have CPU-frequency enabled or
the CPU frequency is changed by means of the CPC core clock divider
available on the platforms with CM3 or newer.
Signed-off-by: Paul Burton <paulburton@kernel.org>
Co-developed-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
[Sergey.Semin@baikalelectronics.ru: Register sched-clock if CM3 or !CPU-freq]
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- Register sched clocks only if MIPS GIC belongs to CM3 or if CPU-freq
isn't supported.
---
drivers/clocksource/mips-gic-timer.c | 31 ++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 8b5f8ae723cb..ef12c12c2432 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -16,6 +16,7 @@
#include <linux/notifier.h>
#include <linux/of_irq.h>
#include <linux/percpu.h>
+#include <linux/sched_clock.h>
#include <linux/smp.h>
#include <linux/time.h>
#include <asm/mips-cps.h>
@@ -24,13 +25,10 @@ static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
static int gic_timer_irq;
static unsigned int gic_frequency;
-static u64 notrace gic_read_count(void)
+static u64 notrace gic_read_count_2x32(void)
{
unsigned int hi, hi2, lo;
- if (mips_cm_is64)
- return read_gic_counter();
-
do {
hi = read_gic_counter_32h();
lo = read_gic_counter_32l();
@@ -40,6 +38,19 @@ static u64 notrace gic_read_count(void)
return (((u64) hi) << 32) + lo;
}
+static u64 notrace gic_read_count_64(void)
+{
+ return read_gic_counter();
+}
+
+static u64 notrace gic_read_count(void)
+{
+ if (mips_cm_is64)
+ return gic_read_count_64();
+
+ return gic_read_count_2x32();
+}
+
static int gic_next_event(unsigned long delta, struct clock_event_device *evt)
{
int cpu = cpumask_first(evt->cpumask);
@@ -228,6 +239,18 @@ static int __init gic_clocksource_of_init(struct device_node *node)
/* And finally start the counter */
clear_gic_config(GIC_CONFIG_COUNTSTOP);
+ /*
+ * It's safe to use the MIPS GIC timer as a sched clock source only if
+ * its ticks are stable, which is true on either the platforms with
+ * stable CPU frequency or on the platforms with CM3 and CPU frequency
+ * change performed by the CPC core clocks divider.
+ */
+ if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
+ sched_clock_register(mips_cm_is64 ?
+ gic_read_count_64 : gic_read_count_2x32,
+ 64, gic_frequency);
+ }
+
return 0;
}
TIMER_OF_DECLARE(mips_gic_timer, "mti,gic-timer",
--
2.25.1
^ permalink raw reply related
* [PATCH v5 8/8] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Paul Cercueil,
Claudiu Beznea, Krzysztof Kozlowski, Maarten ter Huurne,
Bartosz Golaszewski, Randy Dunlap, Vincenzo Frascino,
linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently clocksource framework doesn't support the clocks with variable
frequency. Since MIPS GIC timer ticks rate might be unstable on some
platforms, we must make sure that it justifies the clocksource
requirements. MIPS GIC timer is incremented with the CPU cluster reference
clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
changes synchronously. Due to this the clocksource subsystem can't rely on
the timer to measure system clocks anymore. This commit marks the MIPS GIC
based clocksource as unstable if reference clock (normally it's a CPU
reference clocks) rate changes. The clocksource will execute a watchdog
thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
new stable one.
Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
GIC clocksource since normally the timer is stable. The only reason why
it gets unstable is due to the ref clock rate change, which event we
detect here in the driver by means of the clocks event notifier.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v4:
- Mark clocksource as unstable instead of lowering its rating.
Changelog v5:
- Fix mistakenly added "git_" prefix.
---
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/mips-gic-timer.c | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index f2142e6bbea3..37a745f3ca91 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -572,6 +572,7 @@ config CLKSRC_VERSATILE
config CLKSRC_MIPS_GIC
bool
depends on MIPS_GIC
+ select CLOCKSOURCE_WATCHDOG
select TIMER_OF
config CLKSRC_TANGO_XTAL
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index ef12c12c2432..be4175f415ba 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -24,6 +24,9 @@
static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
static int gic_timer_irq;
static unsigned int gic_frequency;
+static bool __read_mostly gic_clock_unstable;
+
+static void gic_clocksource_unstable(char *reason);
static u64 notrace gic_read_count_2x32(void)
{
@@ -125,8 +128,10 @@ static int gic_clk_notifier(struct notifier_block *nb, unsigned long action,
{
struct clk_notifier_data *cnd = data;
- if (action == POST_RATE_CHANGE)
+ if (action == POST_RATE_CHANGE) {
+ gic_clocksource_unstable("ref clock rate change");
on_each_cpu(gic_update_frequency, (void *)cnd->new_rate, 1);
+ }
return NOTIFY_OK;
}
@@ -172,6 +177,18 @@ static struct clocksource gic_clocksource = {
.vdso_clock_mode = VDSO_CLOCKMODE_GIC,
};
+static void gic_clocksource_unstable(char *reason)
+{
+ if (gic_clock_unstable)
+ return;
+
+ gic_clock_unstable = true;
+
+ pr_info("GIC timer is unstable due to %s\n", reason);
+
+ clocksource_mark_unstable(&gic_clocksource);
+}
+
static int __init __gic_clocksource_init(void)
{
unsigned int count_width;
--
2.25.1
^ permalink raw reply related
* [PATCH v5 5/8] clocksource: dw_apb_timer: Affiliate of-based timer with any CPU
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, Allison Randal,
Greg Kroah-Hartman, Alexios Zavras, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently any DW APB Timer device detected in OF is bound to CPU #0.
Doing so is redundant since DW APB Timer isn't CPU-local timer, but as
having APB interface is normally accessible from any CPU in the system. By
artificially affiliating the DW timer to the very first CPU we may and in
our case will make the clockevent subsystem to decline the more performant
real CPU-local timers selection in favor of in fact non-local and
accessible over a slow bus - DW APB Timers.
Let's not affiliate the of-detected DW APB Timers to any CPU. By doing so
the clockevent framework would prefer to select the real CPU-local timer
instead of DW APB one. Otherwise if there is no other than DW APB device
for clockevents tracking then it will be selected.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v5:
- This is a new patch created after reconsidering the solution of the DW
APB Timer CPU-affiliation problem.
---
drivers/clocksource/dw_apb_timer_of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 8c28b127759f..2db490f35c20 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -73,7 +73,7 @@ static void __init add_clockevent(struct device_node *event_timer)
timer_get_base_and_rate(event_timer, &iobase, &rate);
- ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
+ ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
rate);
if (!ced)
panic("Unable to initialise clockevent device");
--
2.25.1
^ permalink raw reply related
* [PATCH v5 4/8] clocksource: dw_apb_timer: Make CPU-affiliation being optional
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
Rob Herring, linux-mips, linux-rtc, devicetree, afzal mohammed,
Allison Randal, Greg Kroah-Hartman, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Currently the DW APB Timer driver binds each clockevent timers to a
particular CPU. This isn't good for multiple reasons. First of all seeing
the device is placed on APB bus (which makes it accessible from any CPU
core), accessible over MMIO and having the DYNIRQ flag set we can be sure
that manually binding the timer to any CPU just isn't correct. By doing
so we just set an extra limitation on device usage. This also doesn't
reflect the device actual capability, since by setting the IRQ affinity
we can make it virtually local to any CPU. Secondly imagine if you had a
real CPU-local timer with the same rating and the same CPU-affinity.
In this case if DW APB timer was registered first, then due to the
clockevent framework tick-timer selection procedure we'll end up with the
real CPU-local timer being left unselected for clock-events tracking. But
on most of the platforms (MIPS/ARM/etc) such timers are normally embedded
into the CPU core and are accessible with much better performance then
devices placed on APB. For instance in MIPS architectures there is
r4k-timer, which is CPU-local, assigned with the same rating, and normally
its clockevent device is registered after the platform-specific one.
So in order to fix all of these issues let's make the DW APB Timer CPU
affinity being optional and deactivated by passing a negative CPU id,
which will effectively set the DW APB clockevent timer cpumask to
'cpu_possible_mask'.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v5:
- Don't discard CPU affiliation functionality, but allow it being optional.
If CPU id passed to the clockevent registration method is negative, then
don't assign the timer events to any particular CPU.
- Discard the OF-based DW APB Timer CPU-affiliation setting change. It'll
be moved to a dedicated patch.
---
drivers/clocksource/dw_apb_timer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
index b207a77b0831..f5f24a95ee82 100644
--- a/drivers/clocksource/dw_apb_timer.c
+++ b/drivers/clocksource/dw_apb_timer.c
@@ -222,7 +222,8 @@ static int apbt_next_event(unsigned long delta,
/**
* dw_apb_clockevent_init() - use an APB timer as a clock_event_device
*
- * @cpu: The CPU the events will be targeted at.
+ * @cpu: The CPU the events will be targeted at or -1 if CPU affiliation
+ * isn't required.
* @name: The name used for the timer and the IRQ for it.
* @rating: The rating to give the timer.
* @base: I/O base for the timer registers.
@@ -257,7 +258,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
dw_ced->ced.max_delta_ticks = 0x7fffffff;
dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced);
dw_ced->ced.min_delta_ticks = 5000;
- dw_ced->ced.cpumask = cpumask_of(cpu);
+ dw_ced->ced.cpumask = cpu < 0 ? cpu_possible_mask : cpumask_of(cpu);
dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
dw_ced->ced.set_state_shutdown = apbt_shutdown;
--
2.25.1
^ permalink raw reply related
* [PATCH v5 3/8] dt-bindings: interrupt-controller: Convert mti,gic to DT schema
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Jason Cooper, Marc Zyngier, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
Arnd Bergmann, linux-mips, linux-rtc, linux-kernel, devicetree
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces MIPS GIC legacy bare
text binding with YAML file. As before the binding file states that the
corresponding dts node is supposed to be compatible with MIPS Global
Interrupt Controller indicated by the "mti,gic" compatible string and
to provide a mandatory interrupt-controller and '#interrupt-cells'
properties. There might be optional registers memory range,
"mti,reserved-cpu-vectors" and "mti,reserved-ipi-vectors" properties
specified.
MIPS GIC also includes a free-running global timer, per-CPU count/compare
timers, and a watchdog. Since currently the GIC Timer is only supported the
DT schema expects an IRQ and clock-phandler charged timer sub-node with
"mti,mips-gic-timer" compatible string.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
---
I don't really know who is the corresponding driver maintainer, so I
added Paul to the maintainers property since he used to be looking for the
MIPS arch and Thomas looking after it now. Any idea what email should be
specified there instead?
Changelog v3:
- Since timer sub-node has no unit-address, the node shouldn't be named
with one. So alter the MIPS GIC bindings to have a pure "timer"
sub-node.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ].
- Since it's a conversion patch use GPL-2.0-only SPDX header.
---
.../interrupt-controller/mips-gic.txt | 67 --------
.../interrupt-controller/mti,gic.yaml | 148 ++++++++++++++++++
2 files changed, 148 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt b/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
deleted file mode 100644
index 173595305e26..000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-MIPS Global Interrupt Controller (GIC)
-
-The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
-It also supports local (per-processor) interrupts and software-generated
-interrupts which can be used as IPIs. The GIC also includes a free-running
-global timer, per-CPU count/compare timers, and a watchdog.
-
-Required properties:
-- compatible : Should be "mti,gic".
-- interrupt-controller : Identifies the node as an interrupt controller
-- #interrupt-cells : Specifies the number of cells needed to encode an
- interrupt specifier. Should be 3.
- - The first cell is the type of interrupt, local or shared.
- See <include/dt-bindings/interrupt-controller/mips-gic.h>.
- - The second cell is the GIC interrupt number.
- - The third cell encodes the interrupt flags.
- See <include/dt-bindings/interrupt-controller/irq.h> for a list of valid
- flags.
-
-Optional properties:
-- reg : Base address and length of the GIC registers. If not present,
- the base address reported by the hardware GCR_GIC_BASE will be used.
-- mti,reserved-cpu-vectors : Specifies the list of CPU interrupt vectors
- to which the GIC may not route interrupts. Valid values are 2 - 7.
- This property is ignored if the CPU is started in EIC mode.
-- mti,reserved-ipi-vectors : Specifies the range of GIC interrupts that are
- reserved for IPIs.
- It accepts 2 values, the 1st is the starting interrupt and the 2nd is the size
- of the reserved range.
- If not specified, the driver will allocate the last 2 * number of VPEs in the
- system.
-
-Required properties for timer sub-node:
-- compatible : Should be "mti,gic-timer".
-- interrupts : Interrupt for the GIC local timer.
-
-Optional properties for timer sub-node:
-- clocks : GIC timer operating clock.
-- clock-frequency : Clock frequency at which the GIC timers operate.
-
-Note that one of clocks or clock-frequency must be specified.
-
-Example:
-
- gic: interrupt-controller@1bdc0000 {
- compatible = "mti,gic";
- reg = <0x1bdc0000 0x20000>;
-
- interrupt-controller;
- #interrupt-cells = <3>;
-
- mti,reserved-cpu-vectors = <7>;
- mti,reserved-ipi-vectors = <40 8>;
-
- timer {
- compatible = "mti,gic-timer";
- interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
- clock-frequency = <50000000>;
- };
- };
-
- uart@18101400 {
- ...
- interrupt-parent = <&gic>;
- interrupts = <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>;
- ...
- };
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
new file mode 100644
index 000000000000..9f0eb3addac4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/mti,gic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPS Global Interrupt Controller
+
+maintainers:
+ - Paul Burton <paulburton@kernel.org>
+ - Thomas Bogendoerfer <tsbogend@alpha.franken.de>
+
+description: |
+ The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
+ It also supports local (per-processor) interrupts and software-generated
+ interrupts which can be used as IPIs. The GIC also includes a free-running
+ global timer, per-CPU count/compare timers, and a watchdog.
+
+properties:
+ compatible:
+ const: mti,gic
+
+ "#interrupt-cells":
+ const: 3
+ description: |
+ The 1st cell is the type of interrupt: local or shared defined in the
+ file 'dt-bindings/interrupt-controller/mips-gic.h'. The 2nd cell is the
+ GIC interrupt number. The 3d cell encodes the interrupt flags setting up
+ the IRQ trigger modes, which are defined in the file
+ 'dt-bindings/interrupt-controller/irq.h'.
+
+ reg:
+ description: |
+ Base address and length of the GIC registers space. If not present,
+ the base address reported by the hardware GCR_GIC_BASE will be used.
+ maxItems: 1
+
+ interrupt-controller: true
+
+ mti,reserved-cpu-vectors:
+ description: |
+ Specifies the list of CPU interrupt vectors to which the GIC may not
+ route interrupts. This property is ignored if the CPU is started in EIC
+ mode.
+ allOf:
+ - $ref: /schemas/types.yaml#definitions/uint32-array
+ - minItems: 1
+ maxItems: 6
+ uniqueItems: true
+ items:
+ minimum: 2
+ maximum: 7
+
+ mti,reserved-ipi-vectors:
+ description: |
+ Specifies the range of GIC interrupts that are reserved for IPIs.
+ It accepts two values: the 1st is the starting interrupt and the 2nd is
+ the size of the reserved range. If not specified, the driver will
+ allocate the last (2 * number of VPEs in the system).
+ allOf:
+ - $ref: /schemas/types.yaml#definitions/uint32-array
+ - items:
+ - minimum: 0
+ maximum: 254
+ - minimum: 2
+ maximum: 254
+
+ timer:
+ type: object
+ description: |
+ MIPS GIC includes a free-running global timer, per-CPU count/compare
+ timers, and a watchdog. Currently only the GIC Timer is supported.
+ properties:
+ compatible:
+ const: mti,gic-timer
+
+ interrupts:
+ description: |
+ Interrupt for the GIC local timer, so normally it's suppose to be of
+ <GIC_LOCAL X IRQ_TYPE_NONE> format.
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-frequency: true
+
+ required:
+ - compatible
+ - interrupts
+
+ oneOf:
+ - required:
+ - clocks
+ - required:
+ - clock-frequency
+
+ additionalProperties: false
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - "#interrupt-cells"
+ - interrupt-controller
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/mips-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ interrupt-controller@1bdc0000 {
+ compatible = "mti,gic";
+ reg = <0x1bdc0000 0x20000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ mti,reserved-cpu-vectors = <7>;
+ mti,reserved-ipi-vectors = <40 8>;
+
+ timer {
+ compatible = "mti,gic-timer";
+ interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+ clock-frequency = <50000000>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/mips-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ interrupt-controller@1bdc0000 {
+ compatible = "mti,gic";
+ reg = <0x1bdc0000 0x20000>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+
+ timer {
+ compatible = "mti,gic-timer";
+ interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+ clocks = <&cpu_pll>;
+ };
+ };
+ - |
+ interrupt-controller {
+ compatible = "mti,gic";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ };
+...
--
2.25.1
^ permalink raw reply related
* [PATCH v5 2/8] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Alessandro Zummo, Alexandre Belloni, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
This binding file doesn't belong to the rtc seeing it's a pure timer
with no rtc facilities like days/months/years counting and alarms.
So move the YAML-file to the Documentation/devicetree/bindings/timer/
directory.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
.../devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
rename Documentation/devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml (96%)
diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
similarity index 96%
rename from Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
rename to Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
index 002fe1ee709b..5d300efdf0ca 100644
--- a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
%YAML 1.2
---
-$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$id: http://devicetree.org/schemas/timer/snps,dw-apb-timer.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
title: Synopsys DesignWare APB Timer
--
2.25.1
^ permalink raw reply related
* [PATCH v5 1/8] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
Alessandro Zummo, Alexandre Belloni, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20200521204818.25436-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces Synopsys DW Timer
legacy bare text binding with YAML file. As before the binding file
states that the corresponding dts node is supposed to be compatible
with generic DW APB Timer indicated by the "snps,dw-apb-timer"
compatible string and to provide a mandatory registers memory range,
one timer interrupt, either reference clock source or a fixed clock
rate value. It may also have an optional APB bus reference clock
phandle specified.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
This binding file doesn't belong to the bindings/rtc seeing it's a pure
timer with no rtc facilities like days/months/years counting and alarms.
The binding file will be moved to the
"Documentation/devicetree/bindings/timer/" directory in the next patch.
I also don't know who is the corresponding driver maintainer, so I added
Daniel Lezcano to the maintainers schema. Any idea what email should be
specified there instead?
Changelog v3:
- Since it's a conversion patch use GPL-2.0-only SPDX header.
- Replace "additionalProperties: false" property with
"unevaluatedProperties: false".
---
.../devicetree/bindings/rtc/dw-apb.txt | 32 -------
.../bindings/rtc/snps,dw-apb-timer.yaml | 88 +++++++++++++++++++
2 files changed, 88 insertions(+), 32 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
create mode 100644 Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt
deleted file mode 100644
index c703d51abb6c..000000000000
--- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-* Designware APB timer
-
-Required properties:
-- compatible: One of:
- "snps,dw-apb-timer"
- "snps,dw-apb-timer-sp" <DEPRECATED>
- "snps,dw-apb-timer-osc" <DEPRECATED>
-- reg: physical base address of the controller and length of memory mapped
- region.
-- interrupts: IRQ line for the timer.
-- either clocks+clock-names or clock-frequency properties
-
-Optional properties:
-- clocks : list of clock specifiers, corresponding to entries in
- the clock-names property;
-- clock-names : should contain "timer" and "pclk" entries, matching entries
- in the clocks property.
-- clock-frequency: The frequency in HZ of the timer.
-- clock-freq: For backwards compatibility with picoxcell
-
-If using the clock specifiers, the pclk clock is optional, as not all
-systems may use one.
-
-
-Example:
- timer@ffe00000 {
- compatible = "snps,dw-apb-timer";
- interrupts = <0 170 4>;
- reg = <0xffe00000 0x1000>;
- clocks = <&timer_clk>, <&timer_pclk>;
- clock-names = "timer", "pclk";
- };
diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
new file mode 100644
index 000000000000..002fe1ee709b
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB Timer
+
+maintainers:
+ - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+properties:
+ compatible:
+ oneOf:
+ - const: snps,dw-apb-timer
+ - enum:
+ - snps,dw-apb-timer-sp
+ - snps,dw-apb-timer-osc
+ deprecated: true
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ items:
+ - description: Timer ticks reference clock source
+ - description: APB interface clock source
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: timer
+ - const: pclk
+
+ clock-frequency: true
+
+ clock-freq:
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ description: |
+ Has the same meaning as the 'clock-frequency' property - timer clock
+ frequency in HZ, but is defined only for the backwards compatibility
+ with the picoxcell platform.
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+oneOf:
+ - required:
+ - clocks
+ - clock-names
+ - required:
+ - clock-frequency
+ - required:
+ - clock-freq
+
+examples:
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clocks = <&timer_clk>, <&timer_pclk>;
+ clock-names = "timer", "pclk";
+ };
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clocks = <&timer_clk>;
+ clock-names = "timer";
+ };
+ - |
+ timer@ffe00000 {
+ compatible = "snps,dw-apb-timer";
+ interrupts = <0 170 4>;
+ reg = <0xffe00000 0x1000>;
+ clock-frequency = <25000000>;
+ };
+...
--
2.25.1
^ permalink raw reply related
* [PATCH v5 0/8] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
From: Serge Semin @ 2020-05-21 20:48 UTC (permalink / raw)
To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
Alexey Kolotnikov, Paul Burton, Ralf Baechle, Arnd Bergmann,
Alessandro Zummo, Alexandre Belloni, Rob Herring, linux-mips,
linux-rtc, devicetree, linux-kernel
As for all Baikal-T1 SoC related patchsets, which need this, we replaced
the DW APB Timer legacy plain text-based dt-binding file with DT schema.
Similarly the MIPS GIC bindings file is also converted to DT schema seeing
it also defines the MIPS GIC Timer binding.
Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
functionality of two another timers: embedded into the MIPS GIC timer and
three external DW timers available over APB bus. But we can't use them
before the corresponding drivers are properly fixed. First of all DW APB
Timer shouldn't be bound to a single CPU, since as being accessible over
APB they are external with respect to all possible CPUs. Secondly there
might be more than just two DW APB Timers in the system (Baikal-T1 has
three of them), so permit the driver to use one of them as a clocksource
and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
timer as a clocksource so register it in the corresponding subsystem
(the patch has been found in the Paul Burton MIPS repo so I left the
original Signed-off-by attribute). Finally in the same way as r4k timer
the MIPS GIC timer should be used with care when CPUFREQ config is enabled
since in case of CM2 the timer counting depends on the CPU reference clock
frequency while the clocksource subsystem currently doesn't support the
timers with non-stable clock.
This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4
Changelog v2:
- Fix the SoB tags.
- Our corporate email server doesn't change Message-Id anymore, so the
patchset is resubmitted being in the cover-letter-threaded format.
- Convert the "snps,dw-apb-timer" binding to DT schema in a dedicated
patch.
- Convert the "mti,gic" binding to DT schema in a dedicated patch.
Link: https://lore.kernel.org/linux-rtc/20200324174325.14213-1-Sergey.Semin@baikalelectronics.ru
Changelog v3:
- Make the MIPS GIC timer sub-node name not having a unit-address number.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ] from MIPS
GIC bindings.
- Add patch moving the "snps,dw-apb-timer" binding file to the directory
with timers binding files.
Link: https://lore.kernel.org/linux-rtc/20200506214107.25956-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Mark clocksource as unstable instead of lowering its rating.
- Move conditional sched clocks registration to the Paul' patch.
- Add Thomas Gleixner to the patchset To-list to draw his attention to the
patch "dt-bindings: interrupt-controller: Convert mti,gic to DT schema".
Link: https://lore.kernel.org/linux-rtc/20200521005321.12129-1-Sergey.Semin@baikalelectronics.ru/
Changelog v5:
- Fix mistakenly added "git_" prefix. Obviously it was supposed to be gic_.
- Add new patch "clocksource: dw_apb_timer: Affiliate of-based timer with
any CPU"
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Paul Burton (1):
clocksource: mips-gic-timer: Register as sched_clock
Serge Semin (7):
dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
dt-bindings: interrupt-controller: Convert mti,gic to DT schema
clocksource: dw_apb_timer: Make CPU-affiliation being optional
clocksource: dw_apb_timer: Affiliate of-based timer with any CPU
clocksource: dw_apb_timer_of: Fix missing clockevent timers
clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock
changes
.../interrupt-controller/mips-gic.txt | 67 --------
.../interrupt-controller/mti,gic.yaml | 148 ++++++++++++++++++
.../devicetree/bindings/rtc/dw-apb.txt | 32 ----
.../bindings/timer/snps,dw-apb-timer.yaml | 88 +++++++++++
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/dw_apb_timer.c | 5 +-
drivers/clocksource/dw_apb_timer_of.c | 8 +-
drivers/clocksource/mips-gic-timer.c | 50 +++++-
8 files changed, 288 insertions(+), 111 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
create mode 100644 Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
--
2.25.1
^ permalink raw reply
* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Tali Perry @ 2020-05-21 20:47 UTC (permalink / raw)
To: Wolfram Sang
Cc: Andy Shevchenko, Ofer Yehielli, Brendan Higgins, avifishman70,
Tomer Maimon, kfting, Patrick Venture, Nancy Yuen, Benjamin Fair,
Rob Herring, linux-arm-kernel, linux-i2c, OpenBMC Maillist,
devicetree, Linux Kernel Mailing List
In-Reply-To: <20200521203758.GA20150@ninjato>
On Thu, May 21, 2020 at 11:37 PM Wolfram Sang <wsa@the-dreams.de> wrote:
>
>
> > > > I wondered also about DEBUG_FS entries. I can see their value when
> > > > developing the driver. But since this is done now, do they really help a
> > > > user to debug a difficult case? I am not sure, and then I wonder if we
> > > > should have that code in upstream. I am open for discussion, though.
> > >
> > > The user wanted to have health monitor implemented on top of the driver.
> > > The user has 16 channels connected the multiple devices. All are operated
> > > using various daemons in the system. Sometimes the slave devices are power down.
> > > Therefor the user wanted to track the health status of the devices.
> >
> > Ah, then there are these options I have in mind (Wolfram, FYI as well!):
> > 1) push with debugfs as a temporary solution and convert to devlink health protocol [1];
> > 2) drop it and develop devlink_health solution;
> > 3) push debugfs and wait if I²C will gain devlink health support
>
> No need for 2). We can push it now and convert it later. That being
> said, I wonder if [1] is suitable for this driver? Things like NACKs and
> timeouts happen regularly on an I2C bus and are not a state of bad
> health.
>
Agree, having a timeout every now and then is not an issue. The user
is interested
in cases when the number of timeouts\BER\nack\recovery are high.
^ permalink raw reply
* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Wolfram Sang @ 2020-05-21 20:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Tali Perry, Ofer Yehielli, Brendan Higgins, avifishman70,
Tomer Maimon, kfting, Patrick Venture, Nancy Yuen, Benjamin Fair,
Rob Herring, linux-arm-kernel, linux-i2c, OpenBMC Maillist,
devicetree, Linux Kernel Mailing List
In-Reply-To: <20200521145347.GO1634618@smile.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
> > > I wondered also about DEBUG_FS entries. I can see their value when
> > > developing the driver. But since this is done now, do they really help a
> > > user to debug a difficult case? I am not sure, and then I wonder if we
> > > should have that code in upstream. I am open for discussion, though.
> >
> > The user wanted to have health monitor implemented on top of the driver.
> > The user has 16 channels connected the multiple devices. All are operated
> > using various daemons in the system. Sometimes the slave devices are power down.
> > Therefor the user wanted to track the health status of the devices.
>
> Ah, then there are these options I have in mind (Wolfram, FYI as well!):
> 1) push with debugfs as a temporary solution and convert to devlink health protocol [1];
> 2) drop it and develop devlink_health solution;
> 3) push debugfs and wait if I²C will gain devlink health support
No need for 2). We can push it now and convert it later. That being
said, I wonder if [1] is suitable for this driver? Things like NACKs and
timeouts happen regularly on an I2C bus and are not a state of bad
health.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2] arm: dts: am335x-boneblack: add gpio-line-names
From: Drew Fustini @ 2020-05-21 20:09 UTC (permalink / raw)
To: Linus Walleij, Grygorii Strashko, Benoît Cousson,
Tony Lindgren, Rob Herring, Linux-OMAP, devicetree, linux-kernel,
Jason Kridner, Robert Nelson
The BeagleBone Black has P8 and P9 headers [0] which expose many of the
AM3358 ZCZ SoC balls to stacking expansion boards called "capes", or to
other external connections like jumper wires connected to a breadboard.
BeagleBone users will often refer to the "Cape Exanpsion Headers" pin
diagram [1] as it is in the "Bone101" getting started tutorial. [2]
Most of the P8 and P9 header pins can muxed to a GPIO line. The
gpio-line-names describe which P8 or P9 pin that line goes to and the
default mux for that P8 or P9 pin if it is not GPIO.
For example, gpiochip 1 line 0 is connected to P8 header pin 25 (P8_25)
however the default device tree has the corresponding BGA ball (ZCZ U7)
muxed to mmc1_dat0 as it is used for the on-board eMMC chip. For that
GPIO line to be used, one would need to modify the device tree to
disable the eMMC and change the pin mux for that ball to GPIO mode.
Some of the AM3358 ZCZ balls corresponding to GPIO lines are not routed
to a P8 or P9 header, but are instead wired to some peripheral device
like on-board eMMC, HDMI framer IC, or status LEDs. Those names are in
brackets to denote those GPIO lines can not be used.
Some GPIO lines are named "[NC]" as the corresponding balls are not
routed to anything on the PCB.
The goal for these names is to make it easier for a user viewing the
output of gpioinfo to determine which P8 or P9 pin is connected to a
GPIO line. The output of gpioinfo on a BeagleBone Black would be:
gpiochip0 - 32 lines:
line 0: "[ethernet]" unused input active-high
line 1: "[ethernet]" unused input active-high
line 2: "P9_22 [spi0_sclk]" unused input active-high
line 3: "P9_21 [spi0_d0]" unused input active-high
line 4: "P9_18 [spi0_d1]" unused input active-high
line 5: "P9_17 [spi0_cs0]" unused input active-high
line 6: "[sd card]" "cd" input active-low [used]
line 7: "P9_42A [ecappwm0]" unused input active-high
line 8: "P8_35 [hdmi]" unused input active-high
line 9: "P8_33 [hdmi]" unused input active-high
line 10: "P8_31 [hdmi]" unused input active-high
line 11: "P8_32 [hdmi]" unused input active-high
line 12: "P9_20 [i2c2_sda]" unused input active-high
line 13: "P9_19 [i2c2_scl]" unused input active-high
line 14: "P9_26 [uart1_rxd]" unused input active-high
line 15: "P9_24 [uart1_txd]" unused input active-high
line 16: "[ethernet]" unused input active-high
line 17: "[ethernet]" unused input active-high
line 18: "[usb]" unused input active-high
line 19: "[hdmi]" unused input active-high
line 20: "P9_41B" unused input active-high
line 21: "[ethernet]" unused input active-high
line 22: "P8_19 [ehrpwm2a]" unused input active-high
line 23: "P8_13 [ehrpwm2b]" unused input active-high
line 24: "[NC]" unused input active-high
line 25: "[NC]" unused input active-high
line 26: "P8_14" unused input active-high
line 27: "P8_17" unused input active-high
line 28: "[ethernet]" unused input active-high
line 29: "[ethernet]" unused input active-high
line 30: "P9_11 [uart4_rxd]" unused input active-high
line 31: "P9_13 [uart4_txd]" unused input active-high
gpiochip1 - 32 lines:
line 0: "P8_25 [emmc]" unused input active-high
line 1: "[emmc]" unused input active-high
line 2: "P8_5 [emmc]" unused input active-high
line 3: "P8_6 [emmc]" unused input active-high
line 4: "P8_23 [emmc]" unused input active-high
line 5: "P8_22 [emmc]" unused input active-high
line 6: "P8_3 [emmc]" unused input active-high
line 7: "P8_4 [emmc]" unused input active-high
line 8: "[NC]" unused input active-high
line 9: "[NC]" unused input active-high
line 10: "[NC]" unused input active-high
line 11: "[NC]" unused input active-high
line 12: "P8_12" unused input active-high
line 13: "P8_11" unused input active-high
line 14: "P8_16" unused input active-high
line 15: "P8_15" unused input active-high
line 16: "P9_15A" unused input active-high
line 17: "P9_23" unused input active-high
line 18: "P9_14 [ehrpwm1a]" unused input active-high
line 19: "P9_16 [ehrpwm1b]" unused input active-high
line 20: "[emmc]" unused input active-high
line 21: "[usr0 led]" "beaglebone:green:heartbeat" output active-high [used]
line 22: "[usr1 led]" "beaglebone:green:mmc0" output active-high [used]
line 23: "[usr2 led]" "beaglebone:green:usr2" output active-high [used]
line 24: "[usr3 led]" "beaglebone:green:usr3" output active-high [used]
line 25: "[hdmi]" "interrupt" input active-high [used]
line 26: "[usb]" unused input active-high
line 27: "[hdmi audio]" "enable" output active-high [used]
line 28: "P9_12" unused input active-high
line 29: "P8_26" unused input active-high
line 30: "P8_21 [emmc]" unused input active-high
line 31: "P8_20 [emmc]" unused input active-high
gpiochip2 - 32 lines:
line 0: "P9_15B" unused input active-high
line 1: "P8_18" unused input active-high
line 2: "P8_7" unused input active-high
line 3: "P8_8" unused input active-high
line 4: "P8_10" unused input active-high
line 5: "P8_9" unused input active-high
line 6: "P8_45 [hdmi]" unused input active-high
line 7: "P8_46 [hdmi]" unused input active-high
line 8: "P8_43 [hdmi]" unused input active-high
line 9: "P8_44 [hdmi]" unused input active-high
line 10: "P8_41 [hdmi]" unused input active-high
line 11: "P8_42 [hdmi]" unused input active-high
line 12: "P8_39 [hdmi]" unused input active-high
line 13: "P8_40 [hdmi]" unused input active-high
line 14: "P8_37 [hdmi]" unused input active-high
line 15: "P8_38 [hdmi]" unused input active-high
line 16: "P8_36 [hdmi]" unused input active-high
line 17: "P8_34 [hdmi]" unused input active-high
line 18: "[ethernet]" unused input active-high
line 19: "[ethernet]" unused input active-high
line 20: "[ethernet]" unused input active-high
line 21: "[ethernet]" unused input active-high
line 22: "P8_27 [hdmi]" unused input active-high
line 23: "P8_29 [hdmi]" unused input active-high
line 24: "P8_28 [hdmi]" unused input active-high
line 25: "P8_30 [hdmi]" unused input active-high
line 26: "[emmc]" unused input active-high
line 27: "[emmc]" unused input active-high
line 28: "[emmc]" unused input active-high
line 29: "[emmc]" unused input active-high
line 30: "[emmc]" unused input active-high
line 31: "[emmc]" unused input active-high
gpiochip3 - 32 lines:
line 0: "[ethernet]" unused input active-high
line 1: "[ethernet]" unused input active-high
line 2: "[ethernet]" unused input active-high
line 3: "[ethernet]" unused input active-high
line 4: "[ethernet]" unused input active-high
line 5: "[i2c0]" unused input active-high
line 6: "[i2c0]" unused input active-high
line 7: "[emu]" unused input active-high
line 8: "[emu]" unused input active-high
line 9: "[ethernet]" unused input active-high
line 10: "[ethernet]" unused input active-high
line 11: "[NC]" unused input active-high
line 12: "[NC]" unused input active-high
line 13: "[usb]" unused input active-high
line 14: "P9_31 [spi1_sclk]" unused input active-high
line 15: "P9_29 [spi1_d0]" unused input active-high
line 16: "P9_30 [spi1_d1]" unused input active-high
line 17: "P9_28 [spi1_cs0]" unused input active-high
line 18: "P9_42B [ecappwm0]" unused input active-high
line 19: "P9_27" unused input active-high
line 20: "P9_41A" unused input active-high
line 21: "P9_25" unused input active-high
line 22: "[NC]" unused input active-high
line 23: "[NC]" unused input active-high
line 24: "[NC]" unused input active-high
line 25: "[NC]" unused input active-high
line 26: "[NC]" unused input active-high
line 27: "[NC]" unused input active-high
line 28: "[NC]" unused input active-high
line 29: "[NC]" unused input active-high
line 30: "[NC]" unused input active-high
line 31: "[NC]" unused input active-high
[0] https://git.io/JfgOd
[1] https://beagleboard.org/capes
[1] https://beagleboard.org/Support/bone101
[2] https://beagleboard.org/static/images/cape-headers.png
Reviewed-by: Jason Kridner <jason@beagleboard.org>
Reviewed-by: Robert Nelson <robertcnelson@gmail.com>
Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
V2 change:
- include output of gpioinfo to illustrate how the gpio-line-names
will appear. My apologies for forgetting to include in V1.
V1 note:
I had posted a patch with these line names for am335x-bone-common.dtsi
but Grygorii Strashko pointed out that the names are not applicable to
all BeagleBone models. Thus I have created this patch to add these
names just for the BeagleBone Black.
arch/arm/boot/dts/am335x-boneblack.dts | 144 +++++++++++++++++++++++++
1 file changed, 144 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index d3928662aed4..5f31e832eb82 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -23,3 +23,147 @@ oppnitro-1000000000 {
opp-supported-hw = <0x06 0x0100>;
};
};
+
+&gpio0 {
+ gpio-line-names =
+ "[ethernet]",
+ "[ethernet]",
+ "P9_22 [spi0_sclk]",
+ "P9_21 [spi0_d0]",
+ "P9_18 [spi0_d1]",
+ "P9_17 [spi0_cs0]",
+ "[sd card]",
+ "P9_42A [ecappwm0]",
+ "P8_35 [hdmi]",
+ "P8_33 [hdmi]",
+ "P8_31 [hdmi]",
+ "P8_32 [hdmi]",
+ "P9_20 [i2c2_sda]",
+ "P9_19 [i2c2_scl]",
+ "P9_26 [uart1_rxd]",
+ "P9_24 [uart1_txd]",
+ "[ethernet]",
+ "[ethernet]",
+ "[usb]",
+ "[hdmi]",
+ "P9_41B",
+ "[ethernet]",
+ "P8_19 [ehrpwm2a]",
+ "P8_13 [ehrpwm2b]",
+ "[NC]",
+ "[NC]",
+ "P8_14",
+ "P8_17",
+ "[ethernet]",
+ "[ethernet]",
+ "P9_11 [uart4_rxd]",
+ "P9_13 [uart4_txd]";
+};
+
+&gpio1 {
+ gpio-line-names =
+ "P8_25 [emmc]",
+ "[emmc]",
+ "P8_5 [emmc]",
+ "P8_6 [emmc]",
+ "P8_23 [emmc]",
+ "P8_22 [emmc]",
+ "P8_3 [emmc]",
+ "P8_4 [emmc]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "P8_12",
+ "P8_11",
+ "P8_16",
+ "P8_15",
+ "P9_15A",
+ "P9_23",
+ "P9_14 [ehrpwm1a]",
+ "P9_16 [ehrpwm1b]",
+ "[emmc]",
+ "[usr0 led]",
+ "[usr1 led]",
+ "[usr2 led]",
+ "[usr3 led]",
+ "[hdmi]",
+ "[usb]",
+ "[hdmi audio]",
+ "P9_12",
+ "P8_26",
+ "P8_21 [emmc]",
+ "P8_20 [emmc]";
+};
+
+&gpio2 {
+ gpio-line-names =
+ "P9_15B",
+ "P8_18",
+ "P8_7",
+ "P8_8",
+ "P8_10",
+ "P8_9",
+ "P8_45 [hdmi]",
+ "P8_46 [hdmi]",
+ "P8_43 [hdmi]",
+ "P8_44 [hdmi]",
+ "P8_41 [hdmi]",
+ "P8_42 [hdmi]",
+ "P8_39 [hdmi]",
+ "P8_40 [hdmi]",
+ "P8_37 [hdmi]",
+ "P8_38 [hdmi]",
+ "P8_36 [hdmi]",
+ "P8_34 [hdmi]",
+ "[ethernet]",
+ "[ethernet]",
+ "[ethernet]",
+ "[ethernet]",
+ "P8_27 [hdmi]",
+ "P8_29 [hdmi]",
+ "P8_28 [hdmi]",
+ "P8_30 [hdmi]",
+ "[emmc]",
+ "[emmc]",
+ "[emmc]",
+ "[emmc]",
+ "[emmc]",
+ "[emmc]";
+};
+
+&gpio3 {
+ gpio-line-names =
+ "[ethernet]",
+ "[ethernet]",
+ "[ethernet]",
+ "[ethernet]",
+ "[ethernet]",
+ "[i2c0]",
+ "[i2c0]",
+ "[emu]",
+ "[emu]",
+ "[ethernet]",
+ "[ethernet]",
+ "[NC]",
+ "[NC]",
+ "[usb]",
+ "P9_31 [spi1_sclk]",
+ "P9_29 [spi1_d0]",
+ "P9_30 [spi1_d1]",
+ "P9_28 [spi1_cs0]",
+ "P9_42B [ecappwm0]",
+ "P9_27",
+ "P9_41A",
+ "P9_25",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]",
+ "[NC]";
+};
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Sean Anderson @ 2020-05-21 20:05 UTC (permalink / raw)
To: Anup Patel, Palmer Dabbelt, Paul Walmsley, Albert Ou, Rob Herring,
Daniel Lezcano, Thomas Gleixner
Cc: devicetree, Damien Le Moal, Anup Patel, linux-kernel, Atish Patra,
Alistair Francis, linux-riscv
In-Reply-To: <20200521134544.816918-6-anup.patel@wdc.com>
On 5/21/20 9:45 AM, Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
> .../bindings/timer/sifive,clint.txt | 33 +++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> new file mode 100644
> index 000000000000..cae2dad1223a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> @@ -0,0 +1,33 @@
> +SiFive Core Local Interruptor (CLINT)
> +-------------------------------------
> +
> +SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
> +Local Interruptor (CLINT) for M-mode timer and inter-processor interrupts.
> +
> +It directly connects to the timer and inter-processor interrupt lines of
> +various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local interrupt
> +controller is the parent interrupt controller for CLINT device.
> +
> +The clock frequency of CLINT is specified via "timebase-frequency" DT
> +property of "/cpus" DT node. The "timebase-frequency" DT property is
> +described in: Documentation/devicetree/bindings/riscv/cpus.yaml
> +
> +Required properties:
> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> + detailed implementation in case that specific bugs need to be worked around.
Should the "riscv,clint0" compatible string be documented here? This
peripheral is not really specific to sifive, as it is present in most
rocket-chip cores.
> +- reg : Should contain 1 register range (address and length).
> +- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
> + the CLINT. Each node pointed to should be a riscv,cpu-intc node, which
> + has a riscv node as parent.
> +
> +Example:
> +
> + clint@2000000 {
> + compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
> + interrupts-extended = <
> + &cpu1-intc 3 &cpu1-intc 7
> + &cpu2-intc 3 &cpu2-intc 7
> + &cpu3-intc 3 &cpu3-intc 7
> + &cpu4-intc 3 &cpu4-intc 7>;
> + reg = <0x2000000 0x4000000>;
> + };
>
--Sean
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: mfd: Add ENE KB3930 Embedded Controller binding
From: robh @ 2020-05-21 20:00 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Mark Rutland, Lee Jones, linux-kernel, Rob Herring, devicetree
In-Reply-To: <20200521115407.2249702-2-lkundrak@v3.sk>
On Thu, 21 May 2020 13:54:06 +0200, Lubomir Rintel wrote:
> Add binding document for the ENE KB3930 Embedded Controller.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> ---
> Changes since v1:
> - Addressed binding validation failure
> ---
> .../devicetree/bindings/mfd/ene-kb3930.yaml | 55 +++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/ene-kb3930.yaml
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
^ permalink raw reply
* Re: [PATCH v8 5/5] dt-bindings: arm: fsl: add different Protonic boards
From: robh @ 2020-05-21 20:00 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Shawn Guo, Mark Rutland, Fabio Estevam, devicetree,
linux-arm-kernel, Sascha Hauer, Pengutronix Kernel Team,
NXP Linux Team, Rob Herring, linux-kernel
In-Reply-To: <20200520154116.12909-6-o.rempel@pengutronix.de>
On Wed, 20 May 2020 17:41:16 +0200, Oleksij Rempel wrote:
> Add Protonic PRTI6Q, WD2, RVT, VT7 boards.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> Documentation/devicetree/bindings/arm/fsl.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
^ permalink raw reply
* Re: [PATCH v12 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
From: robh @ 2020-05-21 19:58 UTC (permalink / raw)
To: Tali Perry
Cc: avifishman70, linux-i2c, linux-arm-kernel, yuenn, wsa,
andriy.shevchenko, devicetree, kfting, venture, brendanhiggins,
ofery, benjaminfair, openbmc, tmaimon77, robh+dt, linux-kernel
In-Reply-To: <20200521110910.45518-2-tali.perry1@gmail.com>
On Thu, 21 May 2020 14:09:08 +0300, Tali Perry wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM I2C controller.
>
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
> ---
> .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Error: Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.example.dts:22.28-29 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1295115
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
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