Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 03/21] ARM: Kirkwood: Convert mv88f6281gtw_ge switch setup to DT
From: Andrew Lunn @ 2014-02-07  9:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207092505.4a0e9963@skate>

On Fri, Feb 07, 2014 at 09:25:05AM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Fri,  7 Feb 2014 00:41:59 +0100, Andrew Lunn wrote:
> > The mv88f6281gtw_ge has a ethernet switch connected to the ethernet
> > port of the SoC. Convert the platform device instantiationn to a DT
> > instantiation.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  arch/arm/mach-kirkwood/Kconfig                 |  7 ----
> >  arch/arm/mach-kirkwood/Makefile                |  1 -
> >  arch/arm/mach-kirkwood/Module.symvers          |  0
> >  arch/arm/mach-kirkwood/board-dt.c              |  3 --
> >  arch/arm/mach-kirkwood/board-mv88f6281gtw_ge.c | 50 --------------------------
> >  arch/arm/mach-kirkwood/common.h                |  7 ----
> >  6 files changed, 68 deletions(-)
> >  create mode 100644 arch/arm/mach-kirkwood/Module.symvers
> 
> This is probably a mistake.
> 
> >  delete mode 100644 arch/arm/mach-kirkwood/board-mv88f6281gtw_ge.c
> 
> Hum, I maybe missed something, but where are the corresponding DT
> informations added to replace the C description of the Ethernet switch?

Hi Thomas

Yes, i screwed that patch up somewhere.

It will be fixed in v2.

   Andrew

^ permalink raw reply

* [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
From: Daniel Vetter @ 2014-02-07  9:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <E1Vypo6-0007FF-Lb@rmk-PC.arm.linux.org.uk>

I've chatted a bit with Hans Verkuil about this topic at fosdem and
apparently both v4l and alsa have something like this already in their
helper libraries. Adding more people as fyi in case they want to
switch to the new driver core stuff from Russell.
-Daniel

On Thu, Jan 2, 2014 at 10:27 PM, Russell King
<rmk+kernel@arm.linux.org.uk> wrote:
> Subsystems such as ALSA, DRM and others require a single card-level
> device structure to represent a subsystem.  However, firmware tends to
> describe the individual devices and the connections between them.
>
> Therefore, we need a way to gather up the individual component devices
> together, and indicate when we have all the component devices.
>
> We do this in DT by providing a "superdevice" node which specifies
> the components, eg:
>
>         imx-drm {
>                 compatible = "fsl,drm";
>                 crtcs = <&ipu1>;
>                 connectors = <&hdmi>;
>         };
>
> The superdevice is declared into the component support, along with the
> subcomponents.  The superdevice receives callbacks to locate the
> subcomponents, and identify when all components are present.  At this
> point, we bind the superdevice, which causes the appropriate subsystem
> to be initialised in the conventional way.
>
> When any of the components or superdevice are removed from the system,
> we unbind the superdevice, thereby taking the subsystem down.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/base/Makefile     |    2 +-
>  drivers/base/component.c  |  379 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/component.h |   31 ++++
>  3 files changed, 411 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/base/component.c
>  create mode 100644 include/linux/component.h
>
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 94e8a80e87f8..870ecfd503af 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -1,6 +1,6 @@
>  # Makefile for the Linux device tree
>
> -obj-y                  := core.o bus.o dd.o syscore.o \
> +obj-y                  := component.o core.o bus.o dd.o syscore.o \
>                            driver.o class.o platform.o \
>                            cpu.o firmware.o init.o map.o devres.o \
>                            attribute_container.o transport_class.o \
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> new file mode 100644
> index 000000000000..5492cd8d2247
> --- /dev/null
> +++ b/drivers/base/component.c
> @@ -0,0 +1,379 @@
> +/*
> + * Componentized device handling.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This is work in progress.  We gather up the component devices into a list,
> + * and bind them when instructed.  At the moment, we're specific to the DRM
> + * subsystem, and only handles one master device, but this doesn't have to be
> + * the case.
> + */
> +#include <linux/component.h>
> +#include <linux/device.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +struct master {
> +       struct list_head node;
> +       struct list_head components;
> +       bool bound;
> +
> +       const struct component_master_ops *ops;
> +       struct device *dev;
> +};
> +
> +struct component {
> +       struct list_head node;
> +       struct list_head master_node;
> +       struct master *master;
> +       bool bound;
> +
> +       const struct component_ops *ops;
> +       struct device *dev;
> +};
> +
> +static DEFINE_MUTEX(component_mutex);
> +static LIST_HEAD(component_list);
> +static LIST_HEAD(masters);
> +
> +static struct master *__master_find(struct device *dev, const struct component_master_ops *ops)
> +{
> +       struct master *m;
> +
> +       list_for_each_entry(m, &masters, node)
> +               if (m->dev == dev && (!ops || m->ops == ops))
> +                       return m;
> +
> +       return NULL;
> +}
> +
> +/* Attach an unattached component to a master. */
> +static void component_attach_master(struct master *master, struct component *c)
> +{
> +       c->master = master;
> +
> +       list_add_tail(&c->master_node, &master->components);
> +}
> +
> +/* Detach a component from a master. */
> +static void component_detach_master(struct master *master, struct component *c)
> +{
> +       list_del(&c->master_node);
> +
> +       c->master = NULL;
> +}
> +
> +int component_master_add_child(struct master *master,
> +       int (*compare)(struct device *, void *), void *compare_data)
> +{
> +       struct component *c;
> +       int ret = -ENXIO;
> +
> +       list_for_each_entry(c, &component_list, node) {
> +               if (c->master)
> +                       continue;
> +
> +               if (compare(c->dev, compare_data)) {
> +                       component_attach_master(master, c);
> +                       ret = 0;
> +                       break;
> +               }
> +       }
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(component_master_add_child);
> +
> +/* Detach all attached components from this master */
> +static void master_remove_components(struct master *master)
> +{
> +       while (!list_empty(&master->components)) {
> +               struct component *c = list_first_entry(&master->components,
> +                                       struct component, master_node);
> +
> +               WARN_ON(c->master != master);
> +
> +               component_detach_master(master, c);
> +       }
> +}
> +
> +/*
> + * Try to bring up a master.  If component is NULL, we're interested in
> + * this master, otherwise it's a component which must be present to try
> + * and bring up the master.
> + *
> + * Returns 1 for successful bringup, 0 if not ready, or -ve errno.
> + */
> +static int try_to_bring_up_master(struct master *master,
> +       struct component *component)
> +{
> +       int ret = 0;
> +
> +       if (!master->bound) {
> +               /*
> +                * Search the list of components, looking for components that
> +                * belong to this master, and attach them to the master.
> +                */
> +               if (master->ops->add_components(master->dev, master)) {
> +                       /* Failed to find all components */
> +                       master_remove_components(master);
> +                       ret = 0;
> +                       goto out;
> +               }
> +
> +               if (component && component->master != master) {
> +                       master_remove_components(master);
> +                       ret = 0;
> +                       goto out;
> +               }
> +
> +               /* Found all components */
> +               ret = master->ops->bind(master->dev);
> +               if (ret < 0) {
> +                       master_remove_components(master);
> +                       goto out;
> +               }
> +
> +               master->bound = true;
> +               ret = 1;
> +       }
> +out:
> +
> +       return ret;
> +}
> +
> +static int try_to_bring_up_masters(struct component *component)
> +{
> +       struct master *m;
> +       int ret = 0;
> +
> +       list_for_each_entry(m, &masters, node) {
> +               ret = try_to_bring_up_master(m, component);
> +               if (ret != 0)
> +                       break;
> +       }
> +
> +       return ret;
> +}
> +
> +static void take_down_master(struct master *master)
> +{
> +       if (master->bound) {
> +               master->ops->unbind(master->dev);
> +               master->bound = false;
> +       }
> +
> +       master_remove_components(master);
> +}
> +
> +int component_master_add(struct device *dev, const struct component_master_ops *ops)
> +{
> +       struct master *master;
> +       int ret;
> +
> +       master = kzalloc(sizeof(*master), GFP_KERNEL);
> +       if (!master)
> +               return -ENOMEM;
> +
> +       master->dev = dev;
> +       master->ops = ops;
> +       INIT_LIST_HEAD(&master->components);
> +
> +       /* Add to the list of available masters. */
> +       mutex_lock(&component_mutex);
> +       list_add(&master->node, &masters);
> +
> +       ret = try_to_bring_up_master(master, NULL);
> +
> +       if (ret < 0) {
> +               /* Delete off the list if we weren't successful */
> +               list_del(&master->node);
> +               kfree(master);
> +       }
> +       mutex_unlock(&component_mutex);
> +
> +       return ret < 0 ? ret : 0;
> +}
> +EXPORT_SYMBOL_GPL(component_master_add);
> +
> +void component_master_del(struct device *dev, const struct component_master_ops *ops)
> +{
> +       struct master *master;
> +
> +       mutex_lock(&component_mutex);
> +       master = __master_find(dev, ops);
> +       if (master) {
> +               take_down_master(master);
> +
> +               list_del(&master->node);
> +               kfree(master);
> +       }
> +       mutex_unlock(&component_mutex);
> +}
> +EXPORT_SYMBOL_GPL(component_master_del);
> +
> +static void component_unbind(struct component *component,
> +       struct master *master, void *data)
> +{
> +       WARN_ON(!component->bound);
> +
> +       component->ops->unbind(component->dev, master->dev, data);
> +       component->bound = false;
> +
> +       /* Release all resources claimed in the binding of this component */
> +       devres_release_group(component->dev, component);
> +}
> +
> +void component_unbind_all(struct device *master_dev, void *data)
> +{
> +       struct master *master;
> +       struct component *c;
> +
> +       WARN_ON(!mutex_is_locked(&component_mutex));
> +
> +       master = __master_find(master_dev, NULL);
> +       if (!master)
> +               return;
> +
> +       list_for_each_entry_reverse(c, &master->components, master_node)
> +               component_unbind(c, master, data);
> +}
> +EXPORT_SYMBOL_GPL(component_unbind_all);
> +
> +static int component_bind(struct component *component, struct master *master,
> +       void *data)
> +{
> +       int ret;
> +
> +       /*
> +        * Each component initialises inside its own devres group.
> +        * This allows us to roll-back a failed component without
> +        * affecting anything else.
> +        */
> +       if (!devres_open_group(master->dev, NULL, GFP_KERNEL))
> +               return -ENOMEM;
> +
> +       /*
> +        * Also open a group for the device itself: this allows us
> +        * to release the resources claimed against the sub-device
> +        * at the appropriate moment.
> +        */
> +       if (!devres_open_group(component->dev, component, GFP_KERNEL)) {
> +               devres_release_group(master->dev, NULL);
> +               return -ENOMEM;
> +       }
> +
> +       dev_dbg(master->dev, "binding %s (ops %ps)\n",
> +               dev_name(component->dev), component->ops);
> +
> +       ret = component->ops->bind(component->dev, master->dev, data);
> +       if (!ret) {
> +               component->bound = true;
> +
> +               /*
> +                * Close the component device's group so that resources
> +                * allocated in the binding are encapsulated for removal
> +                * at unbind.  Remove the group on the DRM device as we
> +                * can clean those resources up independently.
> +                */
> +               devres_close_group(component->dev, NULL);
> +               devres_remove_group(master->dev, NULL);
> +
> +               dev_info(master->dev, "bound %s (ops %ps)\n",
> +                        dev_name(component->dev), component->ops);
> +       } else {
> +               devres_release_group(component->dev, NULL);
> +               devres_release_group(master->dev, NULL);
> +
> +               dev_err(master->dev, "failed to bind %s (ops %ps): %d\n",
> +                       dev_name(component->dev), component->ops, ret);
> +       }
> +
> +       return ret;
> +}
> +
> +int component_bind_all(struct device *master_dev, void *data)
> +{
> +       struct master *master;
> +       struct component *c;
> +       int ret = 0;
> +
> +       WARN_ON(!mutex_is_locked(&component_mutex));
> +
> +       master = __master_find(master_dev, NULL);
> +       if (!master)
> +               return -EINVAL;
> +
> +       list_for_each_entry(c, &master->components, master_node) {
> +               ret = component_bind(c, master, data);
> +               if (ret)
> +                       break;
> +       }
> +
> +       if (ret != 0) {
> +               list_for_each_entry_continue_reverse(c, &master->components,
> +                                                    master_node)
> +                       component_unbind(c, master, data);
> +       }
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(component_bind_all);
> +
> +int component_add(struct device *dev, const struct component_ops *ops)
> +{
> +       struct component *component;
> +       int ret;
> +
> +       component = kzalloc(sizeof(*component), GFP_KERNEL);
> +       if (!component)
> +               return -ENOMEM;
> +
> +       component->ops = ops;
> +       component->dev = dev;
> +
> +       dev_dbg(dev, "adding component (ops %ps)\n", ops);
> +
> +       mutex_lock(&component_mutex);
> +       list_add_tail(&component->node, &component_list);
> +
> +       ret = try_to_bring_up_masters(component);
> +       if (ret < 0) {
> +               list_del(&component->node);
> +
> +               kfree(component);
> +       }
> +       mutex_unlock(&component_mutex);
> +
> +       return ret < 0 ? ret : 0;
> +}
> +EXPORT_SYMBOL_GPL(component_add);
> +
> +void component_del(struct device *dev, const struct component_ops *ops)
> +{
> +       struct component *c, *component = NULL;
> +
> +       mutex_lock(&component_mutex);
> +       list_for_each_entry(c, &component_list, node)
> +               if (c->dev == dev && c->ops == ops) {
> +                       list_del(&c->node);
> +                       component = c;
> +                       break;
> +               }
> +
> +       if (component && component->master)
> +               take_down_master(component->master);
> +
> +       mutex_unlock(&component_mutex);
> +
> +       WARN_ON(!component);
> +       kfree(component);
> +}
> +EXPORT_SYMBOL_GPL(component_del);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/component.h b/include/linux/component.h
> new file mode 100644
> index 000000000000..73657636db0b
> --- /dev/null
> +++ b/include/linux/component.h
> @@ -0,0 +1,31 @@
> +#ifndef COMPONENT_H
> +#define COMPONENT_H
> +
> +struct device;
> +
> +struct component_ops {
> +       int (*bind)(struct device *, struct device *, void *);
> +       void (*unbind)(struct device *, struct device *, void *);
> +};
> +
> +int component_add(struct device *, const struct component_ops *);
> +void component_del(struct device *, const struct component_ops *);
> +
> +int component_bind_all(struct device *, void *);
> +void component_unbind_all(struct device *, void *);
> +
> +struct master;
> +
> +struct component_master_ops {
> +       int (*add_components)(struct device *, struct master *);
> +       int (*bind)(struct device *);
> +       void (*unbind)(struct device *);
> +};
> +
> +int component_master_add(struct device *, const struct component_master_ops *);
> +void component_master_del(struct device *, const struct component_master_ops *);
> +
> +int component_master_add_child(struct master *master,
> +       int (*compare)(struct device *, void *), void *compare_data);
> +
> +#endif
> --
> 1.7.4.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* [PATCH v4 02/10] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation
From: Jungseok Lee @ 2014-02-07  9:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAhSdy3Y3N7pSr3Y0rB7p340sX_kGPFT7mmBa2Rvf2pdSJYAfA@mail.gmail.com>

On Friday, February 07, 2014 5:36 PM, Anup Patel wrote:
> On Fri, Feb 7, 2014 at 1:58 PM, Jungseok Lee <jays.lee@samsung.com> wrote:
> > On Thursday, February 06, 2014 8:32 PM, Anup Patel wrote:
> >> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface
> >> to VCPUs. This patch extends current in-kernel PSCI emulation to provide PSCI v0.2 interface to
> VCPUs.
> >>
> >> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
> >> keeping the ABI backward- compatible.
> >>
> >> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
> >> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing
> >> VCPU init using KVM_ARM_VCPU_INIT ioctl.
> >>
> >> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >> ---
> >>  arch/arm/include/asm/kvm_host.h   |    2 +-
> >>  arch/arm/include/asm/kvm_psci.h   |    4 ++
> >>  arch/arm/include/uapi/asm/kvm.h   |   35 ++++++++++++++-
> >>  arch/arm/kvm/arm.c                |    1 +
> >>  arch/arm/kvm/psci.c               |   85 +++++++++++++++++++++++++++++++------
> >>  arch/arm64/include/asm/kvm_host.h |    2 +-
> >>  arch/arm64/include/asm/kvm_psci.h |    4 ++
> >>  arch/arm64/include/uapi/asm/kvm.h |   35 ++++++++++++++-
> >>  8 files changed, 152 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/kvm_host.h
> >> b/arch/arm/include/asm/kvm_host.h index 09af149..193ceaf
> >> 100644
> >> --- a/arch/arm/include/asm/kvm_host.h
> >> +++ b/arch/arm/include/asm/kvm_host.h
> >> @@ -36,7 +36,7 @@
> >>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1  #define KVM_HAVE_ONE_REG
> >>
> >> -#define KVM_VCPU_MAX_FEATURES 1
> >> +#define KVM_VCPU_MAX_FEATURES 2
> >>
> >>  #include <kvm/arm_vgic.h>
> >>
> >> diff --git a/arch/arm/include/asm/kvm_psci.h
> >> b/arch/arm/include/asm/kvm_psci.h index 9a83d98..4c0e3e1
> >> 100644
> >> --- a/arch/arm/include/asm/kvm_psci.h
> >> +++ b/arch/arm/include/asm/kvm_psci.h
> >> @@ -18,6 +18,10 @@
> >>  #ifndef __ARM_KVM_PSCI_H__
> >>  #define __ARM_KVM_PSCI_H__
> >>
> >> +#define KVM_ARM_PSCI_0_1     1
> >> +#define KVM_ARM_PSCI_0_2     2
> >> +
> >> +int kvm_psci_version(struct kvm_vcpu *vcpu);
> >>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
> >>
> >>  #endif /* __ARM_KVM_PSCI_H__ */
> >> diff --git a/arch/arm/include/uapi/asm/kvm.h
> >> b/arch/arm/include/uapi/asm/kvm.h index ef0c878..9c922d9
> >> 100644
> >> --- a/arch/arm/include/uapi/asm/kvm.h
> >> +++ b/arch/arm/include/uapi/asm/kvm.h
> >> @@ -83,6 +83,7 @@ struct kvm_regs {
> >>  #define KVM_VGIC_V2_CPU_SIZE         0x2000
> >>
> >>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
> >> +#define KVM_ARM_VCPU_PSCI_0_2                1 /* CPU uses PSCI v0.2 */
> >>
> >>  struct kvm_vcpu_init {
> >>       __u32 target;
> >> @@ -192,7 +193,7 @@ struct kvm_arch_memory_slot {
> >>  /* Highest supported SPI, from VGIC_NR_IRQS */
> >>  #define KVM_ARM_IRQ_GIC_MAX          127
> >>
> >> -/* PSCI interface */
> >> +/* PSCI v0.1 interface */
> >>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
> >>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
> >>
> >> @@ -201,9 +202,41 @@ struct kvm_arch_memory_slot {
> >>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
> >>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
> >>
> >> +/* PSCI v0.2 interface */
> >> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
> >> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
> >> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
> >> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
> >> +
> >> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0) #define
> >> +KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
> >> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
> >> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
> >> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
> >> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
> >> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> >> +                                     KVM_PSCI_0_2_FN(6) #define
> >> +KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> >> +                                     KVM_PSCI_0_2_FN(7)
> >> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
> >> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
> >> +
> >> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
> >> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
> >> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
> >> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
> >> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> >> +                                     KVM_PSCI_0_2_FN64(7)
> >> +
> >
> > Hi
> >
> > This patch describes PSCI v0.2 specification well. Instead, I have a
> > question on interface types, not implementation.
> >
> > According to PSCI v0.2 document, it does not cover DVFS.
> > Could I get an idea why DVFS is not supported?
> 
> PSCI v0.2 only describes few mandatory functions required for being PSCI v0.2 compliant. For rest of
> the stuff such as DVFS you will need to define your own platform specific PSCI function.
> (Marc/Mark correct me if I am wrong here?)

It seems that I should change architecture codes if I need other interfaces.
Under this implementation, I should add my own interface to generic codes,
such as, arch/arm/include/asm/kvm_psci.h. However, I think that it is not
acceptable to upstream. If so, is there any place to define a platform
specific PSCI functions?

Best Regards
Jungseok Lee

> Mab be in future some PSCI vX.Y will define standardized PSCI function for DVFS too.
> 
> Regards,
> Anup
> 
> >
> > Best Regards
> > Jungseok Lee
> >
> >> +/* PSCI return values */
> >>  #define KVM_PSCI_RET_SUCCESS         0
> >>  #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
> >>  #define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
> >>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
> >> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
> >> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
> >> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
> >> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
> >> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
> >>
> >>  #endif /* __ARM_KVM_H__ */
> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index
> >> 1d8248e..c8a71df 100644
> >> --- a/arch/arm/kvm/arm.c
> >> +++ b/arch/arm/kvm/arm.c
> >> @@ -197,6 +197,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> >>       case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
> >>       case KVM_CAP_ONE_REG:
> >>       case KVM_CAP_ARM_PSCI:
> >> +     case KVM_CAP_ARM_PSCI_0_2:
> >>               r = 1;
> >>               break;
> >>       case KVM_CAP_COALESCED_MMIO:
> >> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c index
> >> 448f60e..e4ec4af 100644
> >> --- a/arch/arm/kvm/psci.c
> >> +++ b/arch/arm/kvm/psci.c
> >> @@ -85,17 +85,57 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
> >>       return KVM_PSCI_RET_SUCCESS;
> >>  }
> >>
> >> -/**
> >> - * kvm_psci_call - handle PSCI call if r0 value is in range
> >> - * @vcpu: Pointer to the VCPU struct
> >> - *
> >> - * Handle PSCI calls from guests through traps from HVC instructions.
> >> - * The calling convention is similar to SMC calls to the secure
> >> world where
> >> - * the function number is placed in r0 and this function returns
> >> true if the
> >> - * function number specified in r0 is withing the PSCI range, and
> >> false
> >> - * otherwise.
> >> - */
> >> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >> +int kvm_psci_version(struct kvm_vcpu *vcpu) {
> >> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> >> +             return KVM_ARM_PSCI_0_2;
> >> +
> >> +     return KVM_ARM_PSCI_0_1;
> >> +}
> >> +
> >> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu) {
> >> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> >> +     unsigned long val;
> >> +
> >> +     switch (psci_fn) {
> >> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
> >> +             /*
> >> +              * Bits[31:16] = Major Version = 0
> >> +              * Bits[15:0] = Minor Version = 2
> >> +              */
> >> +             val = 2;
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_OFF:
> >> +             kvm_psci_vcpu_off(vcpu);
> >> +             val = KVM_PSCI_RET_SUCCESS;
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_ON:
> >> +     case KVM_PSCI_0_2_FN64_CPU_ON:
> >> +             val = kvm_psci_vcpu_on(vcpu);
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
> >> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> >> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
> >> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
> >> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
> >> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
> >> +     case KVM_PSCI_0_2_FN64_MIGRATE:
> >> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> >> +             val = KVM_PSCI_RET_NI;
> >> +             break;
> >> +     default:
> >> +             return false;
> >> +     }
> >> +
> >> +     *vcpu_reg(vcpu, 0) = val;
> >> +     return true;
> >> +}
> >> +
> >> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> >>  {
> >>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> >>       unsigned long val;
> >> @@ -112,7 +152,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >>       case KVM_PSCI_FN_MIGRATE:
> >>               val = KVM_PSCI_RET_NI;
> >>               break;
> >> -
> >>       default:
> >>               return false;
> >>       }
> >> @@ -120,3 +159,25 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >>       *vcpu_reg(vcpu, 0) = val;
> >>       return true;
> >>  }
> >> +
> >> +/**
> >> + * kvm_psci_call - handle PSCI call if r0 value is in range
> >> + * @vcpu: Pointer to the VCPU struct
> >> + *
> >> + * Handle PSCI calls from guests through traps from HVC instructions.
> >> + * The calling convention is similar to SMC calls to the secure
> >> +world where
> >> + * the function number is placed in r0 and this function returns
> >> +true if the
> >> + * function number specified in r0 is withing the PSCI range, and
> >> +false
> >> + * otherwise.
> >> + */
> >> +bool kvm_psci_call(struct kvm_vcpu *vcpu) {
> >> +     switch (kvm_psci_version(vcpu)) {
> >> +     case KVM_ARM_PSCI_0_2:
> >> +             return kvm_psci_0_2_call(vcpu);
> >> +     case KVM_ARM_PSCI_0_1:
> >> +             return kvm_psci_0_1_call(vcpu);
> >> +     default:
> >> +             return false;
> >> +     };
> >> +}
> >> diff --git a/arch/arm64/include/asm/kvm_host.h
> >> b/arch/arm64/include/asm/kvm_host.h
> >> index 0a1d697..92242ce 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -39,7 +39,7 @@
> >>  #include <kvm/arm_vgic.h>
> >>  #include <kvm/arm_arch_timer.h>
> >>
> >> -#define KVM_VCPU_MAX_FEATURES 2
> >> +#define KVM_VCPU_MAX_FEATURES 3
> >>
> >>  struct kvm_vcpu;
> >>  int kvm_target_cpu(void);
> >> diff --git a/arch/arm64/include/asm/kvm_psci.h
> >> b/arch/arm64/include/asm/kvm_psci.h
> >> index e301a48..e25c658 100644
> >> --- a/arch/arm64/include/asm/kvm_psci.h
> >> +++ b/arch/arm64/include/asm/kvm_psci.h
> >> @@ -18,6 +18,10 @@
> >>  #ifndef __ARM64_KVM_PSCI_H__
> >>  #define __ARM64_KVM_PSCI_H__
> >>
> >> +#define KVM_ARM_PSCI_0_1     1
> >> +#define KVM_ARM_PSCI_0_2     2
> >> +
> >> +int kvm_psci_version(struct kvm_vcpu *vcpu);
> >>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
> >>
> >>  #endif /* __ARM64_KVM_PSCI_H__ */
> >> diff --git a/arch/arm64/include/uapi/asm/kvm.h
> >> b/arch/arm64/include/uapi/asm/kvm.h
> >> index eaf54a3..cadc318 100644
> >> --- a/arch/arm64/include/uapi/asm/kvm.h
> >> +++ b/arch/arm64/include/uapi/asm/kvm.h
> >> @@ -77,6 +77,7 @@ struct kvm_regs {
> >>
> >>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
> >>  #define KVM_ARM_VCPU_EL1_32BIT               1 /* CPU running a 32bit VM */
> >> +#define KVM_ARM_VCPU_PSCI_0_2                2 /* CPU uses PSCI v0.2 */
> >>
> >>  struct kvm_vcpu_init {
> >>       __u32 target;
> >> @@ -177,7 +178,7 @@ struct kvm_arch_memory_slot {
> >>  /* Highest supported SPI, from VGIC_NR_IRQS */
> >>  #define KVM_ARM_IRQ_GIC_MAX          127
> >>
> >> -/* PSCI interface */
> >> +/* PSCI v0.1 interface */
> >>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
> >>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
> >>
> >> @@ -186,10 +187,42 @@ struct kvm_arch_memory_slot {
> >>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
> >>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
> >>
> >> +/* PSCI v0.2 interface */
> >> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
> >> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
> >> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
> >> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
> >> +
> >> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0) #define
> >> +KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
> >> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
> >> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
> >> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
> >> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
> >> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> >> +                                     KVM_PSCI_0_2_FN(6) #define
> >> +KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> >> +                                     KVM_PSCI_0_2_FN(7)
> >> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
> >> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
> >> +
> >> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
> >> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
> >> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
> >> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
> >> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> >> +                                     KVM_PSCI_0_2_FN64(7)
> >> +
> >> +/* PSCI return values */
> >>  #define KVM_PSCI_RET_SUCCESS         0
> >>  #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
> >>  #define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
> >>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
> >> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
> >> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
> >> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
> >> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
> >> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
> >>
> >>  #endif
> >>
> >> --
> >> 1.7.9.5
> >>
> >> _______________________________________________
> >> kvmarm mailing list
> >> kvmarm at lists.cs.columbia.edu
> >> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
> >
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm at lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

^ permalink raw reply

* [PATCH 21/21] ARM: Kirkwood: Remove DT support
From: Andrew Lunn @ 2014-02-07  9:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207093313.1a979a8f@skate>

On Fri, Feb 07, 2014 at 09:33:13AM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Fri,  7 Feb 2014 00:42:17 +0100, Andrew Lunn wrote:
> > Now that all the device tree support is in mach-mvebu, remove it from
> > mach-kirkwood.
> > 
> > Regenerate kirkwood_defconfig, removing all DT support, and a couple
> > of other redundent options have been removed in the process.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  arch/arm/configs/kirkwood_defconfig |   6 -
> 
> So after this series we have a mvebu_defconfig that only builds the
> ARMv7 platforms of mach-mvebu. I believe this may look strange to
> people looking at the code, seeing that Kirkwood DT support is in
> mach-mvebu, but that mvebu_defconfig doesn't work to build a Kirkwood
> kernel.
> 
> I don't really have a solution, except maybe having mvebu_v7_defconfig
> and mvebu_v5_defconfig to build only the Marvell v7 and Marvell v5
> platforms.

Hi Thomas

I also have no solution, other than what you suggested. I think it
also goes against the rules for a platform to have multiple _defconfig
files. We probably do have a good cause to go against the rules
thought. Lets wait and see what ARM SOC maintainers suggest.

	 Andrew

^ permalink raw reply

* [PATCH 16/21] drivers: Enable building of Kirkwood drivers for mach-mvebu
From: Andrew Lunn @ 2014-02-07  9:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207015915.GQ8533@titan.lakedaemon.net>

On Thu, Feb 06, 2014 at 08:59:15PM -0500, Jason Cooper wrote:
> On Fri, Feb 07, 2014 at 12:42:12AM +0100, Andrew Lunn wrote:
> > With the move to mach-mvebu, drivers Kconfig need tweeking to allow
> > the kirkwood specific drivers to be built.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/cpufreq/Kconfig.arm | 2 +-
> >  drivers/cpuidle/Kconfig.arm | 2 +-
> >  drivers/leds/Kconfig        | 4 ++--
> >  drivers/phy/Kconfig         | 2 +-
> >  drivers/thermal/Kconfig     | 2 +-
> >  sound/soc/kirkwood/Kconfig  | 2 +-
> >  6 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index 31297499a60a..de931081fd01 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -113,7 +113,7 @@ config ARM_INTEGRATOR
> >  	  If in doubt, say Y.
> >  
> >  config ARM_KIRKWOOD_CPUFREQ
> > -	def_bool ARCH_KIRKWOOD && OF
> > +	def_bool (ARCH_KIRKWOOD || MACH_KIRKWOOD) && OF
> 
> I agree with creating MACH_KIRKWOOD underneath ARCH_MVEBU earlier in
> this series, but the 'ARCH_KIRKWOOD || MACH_KIRKWOOD' just looks
> confusing.  Also, MACH_KIRKWOOD implies OF so this particular one could
> be reduced to
> 
> 	def_bool MACH_KIRKWOOD

Yes, will do.

Thanks
	Andrew

^ permalink raw reply

* [PATCH 19/21] ARM: MVEBU: Simplifiy headers and make local
From: Andrew Lunn @ 2014-02-07  9:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207093104.7197224d@skate>

On Fri, Feb 07, 2014 at 09:31:04AM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Fri,  7 Feb 2014 00:42:15 +0100, Andrew Lunn wrote:
> > kirkwood is very nearly fully DT. Remove most of the address
> > definitions from the header files and make it a local header file.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  arch/arm/mach-mvebu/include/mach/bridge-regs.h |  85 ---------------
> >  arch/arm/mach-mvebu/include/mach/kirkwood.h    | 142 -------------------------
> >  arch/arm/mach-mvebu/kirkwood-pm.c              |   2 +-
> >  arch/arm/mach-mvebu/kirkwood.c                 |   2 +-
> >  arch/arm/mach-mvebu/kirkwood.h                 |  22 ++++
> >  5 files changed, 24 insertions(+), 229 deletions(-)
> >  delete mode 100644 arch/arm/mach-mvebu/include/mach/bridge-regs.h
> >  delete mode 100644 arch/arm/mach-mvebu/include/mach/kirkwood.h
> >  create mode 100644 arch/arm/mach-mvebu/kirkwood.h
> 
> Is there any particular reason why this isn't part of PATCH 14/21. I
> was very surprised when reading PATCH 14/21 to see a file named
> "bridge-regs.h" containing Kirkwood-specific definitions added to
> mach-mvebu, and so many address constants being defined, while we are
> DT-based only in mach-mvebu.

Hi Thomas

I wanted to keep 14/21 as a simple move of files, doing the minimal
needed to make it boot. The patches that follow then clean up. I found
that simpler to follow what was going on, since there is nothing
hidden in the move patch.

I can however squash these together if that is what people want to
see.

	Andrew

^ permalink raw reply

* [PATCH] ARM: dts: qcom: Add RNG device tree node
From: Stanimir Varbanov @ 2014-02-07  9:23 UTC (permalink / raw)
  To: linux-arm-kernel

Add the necessary DT node to probe the rng driver on
msm8974 platforms.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 arch/arm/boot/dts/qcom-msm8974.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 9e5dadb101eb..ada0e8216c22 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -117,5 +117,12 @@
 			clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
 			clock-names = "core", "iface";
 		};
+
+		rng at f9bff000 {
+			compatible = "qcom,prng";
+			reg = <0xf9bff000 0x200>;
+			clocks = <&gcc GCC_PRNG_AHB_CLK>;
+			clock-names = "core";
+		};
 	};
 };
-- 
1.8.4.4

^ permalink raw reply related

* [PATCH v4 02/10] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation
From: Anup Patel @ 2014-02-07  9:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <003a01cf23e4$07f07af0$17d170d0$@samsung.com>

On Fri, Feb 7, 2014 at 2:37 PM, Jungseok Lee <jays.lee@samsung.com> wrote:
> On Friday, February 07, 2014 5:36 PM, Anup Patel wrote:
>> On Fri, Feb 7, 2014 at 1:58 PM, Jungseok Lee <jays.lee@samsung.com> wrote:
>> > On Thursday, February 06, 2014 8:32 PM, Anup Patel wrote:
>> >> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface
>> >> to VCPUs. This patch extends current in-kernel PSCI emulation to provide PSCI v0.2 interface to
>> VCPUs.
>> >>
>> >> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
>> >> keeping the ABI backward- compatible.
>> >>
>> >> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
>> >> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing
>> >> VCPU init using KVM_ARM_VCPU_INIT ioctl.
>> >>
>> >> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> >> ---
>> >>  arch/arm/include/asm/kvm_host.h   |    2 +-
>> >>  arch/arm/include/asm/kvm_psci.h   |    4 ++
>> >>  arch/arm/include/uapi/asm/kvm.h   |   35 ++++++++++++++-
>> >>  arch/arm/kvm/arm.c                |    1 +
>> >>  arch/arm/kvm/psci.c               |   85 +++++++++++++++++++++++++++++++------
>> >>  arch/arm64/include/asm/kvm_host.h |    2 +-
>> >>  arch/arm64/include/asm/kvm_psci.h |    4 ++
>> >>  arch/arm64/include/uapi/asm/kvm.h |   35 ++++++++++++++-
>> >>  8 files changed, 152 insertions(+), 16 deletions(-)
>> >>
>> >> diff --git a/arch/arm/include/asm/kvm_host.h
>> >> b/arch/arm/include/asm/kvm_host.h index 09af149..193ceaf
>> >> 100644
>> >> --- a/arch/arm/include/asm/kvm_host.h
>> >> +++ b/arch/arm/include/asm/kvm_host.h
>> >> @@ -36,7 +36,7 @@
>> >>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1  #define KVM_HAVE_ONE_REG
>> >>
>> >> -#define KVM_VCPU_MAX_FEATURES 1
>> >> +#define KVM_VCPU_MAX_FEATURES 2
>> >>
>> >>  #include <kvm/arm_vgic.h>
>> >>
>> >> diff --git a/arch/arm/include/asm/kvm_psci.h
>> >> b/arch/arm/include/asm/kvm_psci.h index 9a83d98..4c0e3e1
>> >> 100644
>> >> --- a/arch/arm/include/asm/kvm_psci.h
>> >> +++ b/arch/arm/include/asm/kvm_psci.h
>> >> @@ -18,6 +18,10 @@
>> >>  #ifndef __ARM_KVM_PSCI_H__
>> >>  #define __ARM_KVM_PSCI_H__
>> >>
>> >> +#define KVM_ARM_PSCI_0_1     1
>> >> +#define KVM_ARM_PSCI_0_2     2
>> >> +
>> >> +int kvm_psci_version(struct kvm_vcpu *vcpu);
>> >>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> >>
>> >>  #endif /* __ARM_KVM_PSCI_H__ */
>> >> diff --git a/arch/arm/include/uapi/asm/kvm.h
>> >> b/arch/arm/include/uapi/asm/kvm.h index ef0c878..9c922d9
>> >> 100644
>> >> --- a/arch/arm/include/uapi/asm/kvm.h
>> >> +++ b/arch/arm/include/uapi/asm/kvm.h
>> >> @@ -83,6 +83,7 @@ struct kvm_regs {
>> >>  #define KVM_VGIC_V2_CPU_SIZE         0x2000
>> >>
>> >>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>> >> +#define KVM_ARM_VCPU_PSCI_0_2                1 /* CPU uses PSCI v0.2 */
>> >>
>> >>  struct kvm_vcpu_init {
>> >>       __u32 target;
>> >> @@ -192,7 +193,7 @@ struct kvm_arch_memory_slot {
>> >>  /* Highest supported SPI, from VGIC_NR_IRQS */
>> >>  #define KVM_ARM_IRQ_GIC_MAX          127
>> >>
>> >> -/* PSCI interface */
>> >> +/* PSCI v0.1 interface */
>> >>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>> >>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>> >>
>> >> @@ -201,9 +202,41 @@ struct kvm_arch_memory_slot {
>> >>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>> >>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>> >>
>> >> +/* PSCI v0.2 interface */
>> >> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> >> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> >> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> >> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> >> +
>> >> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0) #define
>> >> +KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> >> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> >> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> >> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> >> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> >> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> >> +                                     KVM_PSCI_0_2_FN(6) #define
>> >> +KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> >> +                                     KVM_PSCI_0_2_FN(7)
>> >> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> >> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> >> +
>> >> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> >> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> >> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> >> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> >> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> >> +                                     KVM_PSCI_0_2_FN64(7)
>> >> +
>> >
>> > Hi
>> >
>> > This patch describes PSCI v0.2 specification well. Instead, I have a
>> > question on interface types, not implementation.
>> >
>> > According to PSCI v0.2 document, it does not cover DVFS.
>> > Could I get an idea why DVFS is not supported?
>>
>> PSCI v0.2 only describes few mandatory functions required for being PSCI v0.2 compliant. For rest of
>> the stuff such as DVFS you will need to define your own platform specific PSCI function.
>> (Marc/Mark correct me if I am wrong here?)
>
> It seems that I should change architecture codes if I need other interfaces.
> Under this implementation, I should add my own interface to generic codes,
> such as, arch/arm/include/asm/kvm_psci.h. However, I think that it is not
> acceptable to upstream. If so, is there any place to define a platform
> specific PSCI functions?

Are you trying to emulate your platform specific DVFS PSCI
function for a VM using KVM ARM/ARM64??

If so then arch/arm/kvm/psci.c is not the place for it. All platform
specific PSCI function are to be emulated from user space. KVM
ARM/ARM64 will only emulate HVC-based mandatory PSCI v0.2
functions.

User space emulation of platform specific PSCI functions are
still a TODO item for KVM ARM/ARM64. It will require a
mechanism in KVM ARM/ARM64 to route SMC-based
PSCI function calls to user space (i.e. QEMU or KVMTOOL).

Regards,
Anup

>
> Best Regards
> Jungseok Lee
>
>> Mab be in future some PSCI vX.Y will define standardized PSCI function for DVFS too.
>>
>> Regards,
>> Anup
>>
>> >
>> > Best Regards
>> > Jungseok Lee
>> >
>> >> +/* PSCI return values */
>> >>  #define KVM_PSCI_RET_SUCCESS         0
>> >>  #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> >>  #define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> >>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> >> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> >> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> >> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> >> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> >> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>> >>
>> >>  #endif /* __ARM_KVM_H__ */
>> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index
>> >> 1d8248e..c8a71df 100644
>> >> --- a/arch/arm/kvm/arm.c
>> >> +++ b/arch/arm/kvm/arm.c
>> >> @@ -197,6 +197,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>> >>       case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>> >>       case KVM_CAP_ONE_REG:
>> >>       case KVM_CAP_ARM_PSCI:
>> >> +     case KVM_CAP_ARM_PSCI_0_2:
>> >>               r = 1;
>> >>               break;
>> >>       case KVM_CAP_COALESCED_MMIO:
>> >> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c index
>> >> 448f60e..e4ec4af 100644
>> >> --- a/arch/arm/kvm/psci.c
>> >> +++ b/arch/arm/kvm/psci.c
>> >> @@ -85,17 +85,57 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>> >>       return KVM_PSCI_RET_SUCCESS;
>> >>  }
>> >>
>> >> -/**
>> >> - * kvm_psci_call - handle PSCI call if r0 value is in range
>> >> - * @vcpu: Pointer to the VCPU struct
>> >> - *
>> >> - * Handle PSCI calls from guests through traps from HVC instructions.
>> >> - * The calling convention is similar to SMC calls to the secure
>> >> world where
>> >> - * the function number is placed in r0 and this function returns
>> >> true if the
>> >> - * function number specified in r0 is withing the PSCI range, and
>> >> false
>> >> - * otherwise.
>> >> - */
>> >> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >> +int kvm_psci_version(struct kvm_vcpu *vcpu) {
>> >> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> >> +             return KVM_ARM_PSCI_0_2;
>> >> +
>> >> +     return KVM_ARM_PSCI_0_1;
>> >> +}
>> >> +
>> >> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu) {
>> >> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> >> +     unsigned long val;
>> >> +
>> >> +     switch (psci_fn) {
>> >> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
>> >> +             /*
>> >> +              * Bits[31:16] = Major Version = 0
>> >> +              * Bits[15:0] = Minor Version = 2
>> >> +              */
>> >> +             val = 2;
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_OFF:
>> >> +             kvm_psci_vcpu_off(vcpu);
>> >> +             val = KVM_PSCI_RET_SUCCESS;
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_ON:
>> >> +     case KVM_PSCI_0_2_FN64_CPU_ON:
>> >> +             val = kvm_psci_vcpu_on(vcpu);
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
>> >> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> >> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
>> >> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
>> >> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
>> >> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
>> >> +     case KVM_PSCI_0_2_FN64_MIGRATE:
>> >> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>> >> +             val = KVM_PSCI_RET_NI;
>> >> +             break;
>> >> +     default:
>> >> +             return false;
>> >> +     }
>> >> +
>> >> +     *vcpu_reg(vcpu, 0) = val;
>> >> +     return true;
>> >> +}
>> >> +
>> >> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>> >>  {
>> >>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> >>       unsigned long val;
>> >> @@ -112,7 +152,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >>       case KVM_PSCI_FN_MIGRATE:
>> >>               val = KVM_PSCI_RET_NI;
>> >>               break;
>> >> -
>> >>       default:
>> >>               return false;
>> >>       }
>> >> @@ -120,3 +159,25 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >>       *vcpu_reg(vcpu, 0) = val;
>> >>       return true;
>> >>  }
>> >> +
>> >> +/**
>> >> + * kvm_psci_call - handle PSCI call if r0 value is in range
>> >> + * @vcpu: Pointer to the VCPU struct
>> >> + *
>> >> + * Handle PSCI calls from guests through traps from HVC instructions.
>> >> + * The calling convention is similar to SMC calls to the secure
>> >> +world where
>> >> + * the function number is placed in r0 and this function returns
>> >> +true if the
>> >> + * function number specified in r0 is withing the PSCI range, and
>> >> +false
>> >> + * otherwise.
>> >> + */
>> >> +bool kvm_psci_call(struct kvm_vcpu *vcpu) {
>> >> +     switch (kvm_psci_version(vcpu)) {
>> >> +     case KVM_ARM_PSCI_0_2:
>> >> +             return kvm_psci_0_2_call(vcpu);
>> >> +     case KVM_ARM_PSCI_0_1:
>> >> +             return kvm_psci_0_1_call(vcpu);
>> >> +     default:
>> >> +             return false;
>> >> +     };
>> >> +}
>> >> diff --git a/arch/arm64/include/asm/kvm_host.h
>> >> b/arch/arm64/include/asm/kvm_host.h
>> >> index 0a1d697..92242ce 100644
>> >> --- a/arch/arm64/include/asm/kvm_host.h
>> >> +++ b/arch/arm64/include/asm/kvm_host.h
>> >> @@ -39,7 +39,7 @@
>> >>  #include <kvm/arm_vgic.h>
>> >>  #include <kvm/arm_arch_timer.h>
>> >>
>> >> -#define KVM_VCPU_MAX_FEATURES 2
>> >> +#define KVM_VCPU_MAX_FEATURES 3
>> >>
>> >>  struct kvm_vcpu;
>> >>  int kvm_target_cpu(void);
>> >> diff --git a/arch/arm64/include/asm/kvm_psci.h
>> >> b/arch/arm64/include/asm/kvm_psci.h
>> >> index e301a48..e25c658 100644
>> >> --- a/arch/arm64/include/asm/kvm_psci.h
>> >> +++ b/arch/arm64/include/asm/kvm_psci.h
>> >> @@ -18,6 +18,10 @@
>> >>  #ifndef __ARM64_KVM_PSCI_H__
>> >>  #define __ARM64_KVM_PSCI_H__
>> >>
>> >> +#define KVM_ARM_PSCI_0_1     1
>> >> +#define KVM_ARM_PSCI_0_2     2
>> >> +
>> >> +int kvm_psci_version(struct kvm_vcpu *vcpu);
>> >>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> >>
>> >>  #endif /* __ARM64_KVM_PSCI_H__ */
>> >> diff --git a/arch/arm64/include/uapi/asm/kvm.h
>> >> b/arch/arm64/include/uapi/asm/kvm.h
>> >> index eaf54a3..cadc318 100644
>> >> --- a/arch/arm64/include/uapi/asm/kvm.h
>> >> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> >> @@ -77,6 +77,7 @@ struct kvm_regs {
>> >>
>> >>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>> >>  #define KVM_ARM_VCPU_EL1_32BIT               1 /* CPU running a 32bit VM */
>> >> +#define KVM_ARM_VCPU_PSCI_0_2                2 /* CPU uses PSCI v0.2 */
>> >>
>> >>  struct kvm_vcpu_init {
>> >>       __u32 target;
>> >> @@ -177,7 +178,7 @@ struct kvm_arch_memory_slot {
>> >>  /* Highest supported SPI, from VGIC_NR_IRQS */
>> >>  #define KVM_ARM_IRQ_GIC_MAX          127
>> >>
>> >> -/* PSCI interface */
>> >> +/* PSCI v0.1 interface */
>> >>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>> >>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>> >>
>> >> @@ -186,10 +187,42 @@ struct kvm_arch_memory_slot {
>> >>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>> >>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>> >>
>> >> +/* PSCI v0.2 interface */
>> >> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> >> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> >> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> >> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> >> +
>> >> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0) #define
>> >> +KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> >> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> >> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> >> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> >> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> >> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> >> +                                     KVM_PSCI_0_2_FN(6) #define
>> >> +KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> >> +                                     KVM_PSCI_0_2_FN(7)
>> >> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> >> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> >> +
>> >> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> >> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> >> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> >> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> >> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> >> +                                     KVM_PSCI_0_2_FN64(7)
>> >> +
>> >> +/* PSCI return values */
>> >>  #define KVM_PSCI_RET_SUCCESS         0
>> >>  #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> >>  #define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> >>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> >> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> >> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> >> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> >> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> >> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>> >>
>> >>  #endif
>> >>
>> >> --
>> >> 1.7.9.5
>> >>
>> >> _______________________________________________
>> >> kvmarm mailing list
>> >> kvmarm at lists.cs.columbia.edu
>> >> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
>> >
>> > _______________________________________________
>> > kvmarm mailing list
>> > kvmarm at lists.cs.columbia.edu
>> > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
>

^ permalink raw reply

* [PATCH v3 06/15] at91: dt: at91sam9261ek: Adds DT entries for the 4 user buttons
From: Jean-Jacques Hiblot @ 2014-02-07  9:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207082752.GV9558@ns203013.ovh.net>

2014-02-07 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 16:57 Thu 23 Jan     , Jean-Jacques Hiblot wrote:
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> ---
>>  arch/arm/boot/dts/at91sam9261ek.dts | 39 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>
> do only one patch for the 9261ek support no nned to clean
>>
>> diff --git a/arch/arm/boot/dts/at91sam9261ek.dts b/arch/arm/boot/dts/at91sam9261ek.dts
>> index 8909217..5555e9f5 100644
>> --- a/arch/arm/boot/dts/at91sam9261ek.dts
>> +++ b/arch/arm/boot/dts/at91sam9261ek.dts
>> @@ -83,6 +83,15 @@
>>                                               AT91_PIOA 23  AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
>>                                       };
>>                               };
>> +
>> +                             keys {
>> +                                     pinctrl_keys: keys-0 {
>> +                                             atmel,pins = <AT91_PIOA 27  AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> +                                             AT91_PIOA 26  AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> +                                             AT91_PIOA 25  AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> +                                             AT91_PIOA 24  AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
>> +                                     };
>> +                             };
>
> no need this you can drop it
ok. I thought that it would help the user to understand the GPIO usage.
I'll remove all pinmux for GPIO that don't require a special hardware
configuration
>
> you just describe a gpio which we do not describe in pinctrl
>>                       };
>>
>>                       watchdog at fffffd40 {
>> @@ -109,4 +118,34 @@
>>                       linux,default-trigger = "heartbeat";
>>               };
>>       };
>> +
>> +     gpio_keys {
>> +             compatible = "gpio-keys";
>> +             pinctrl-0 = <&pinctrl_keys>;
>> +
>> +             button_0 {
>> +                     label = "button_0";
>> +                     gpios = <&pioA 27 GPIO_ACTIVE_LOW>;
>> +                     linux,code = <256>;
>> +                     gpio-key,wakeup;
>> +             };
>> +             button_1 {
>> +                     label = "button_1";
>> +                     gpios = <&pioA 26 GPIO_ACTIVE_LOW>;
>> +                     linux,code = <257>;
>> +                     gpio-key,wakeup;
>> +             };
>> +             button_2 {
>> +                     label = "button_2";
>> +                     gpios = <&pioA 25 GPIO_ACTIVE_LOW>;
>> +                     linux,code = <258>;
>> +                     gpio-key,wakeup;
>> +             };
>> +             button_3 {
>> +                     label = "button_3";
>> +                     gpios = <&pioA 24 GPIO_ACTIVE_LOW>;
>> +                     linux,code = <259>;
>> +                     gpio-key,wakeup;
>> +             };
>> +     };
>>  };
>> --
>> 1.8.5.2
>>

^ permalink raw reply

* [PATCH 10/21] ARM: MM: Add DT binding for Feroceon L2 cache
From: Andrew Lunn @ 2014-02-07  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207012752.GO8533@titan.lakedaemon.net>

On Thu, Feb 06, 2014 at 08:27:52PM -0500, Jason Cooper wrote:
> On Fri, Feb 07, 2014 at 12:42:06AM +0100, Andrew Lunn wrote:
> > Instantiate the L2 cache from DT. Indicate in DT where the cache
> > control register is and if write through should be made.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  .../devicetree/bindings/arm/mrvl/foroceon.txt      | 19 +++++++++
> >  arch/arm/boot/dts/kirkwood.dtsi                    |  5 +++
> >  arch/arm/include/asm/hardware/cache-feroceon-l2.h  |  2 +
> >  arch/arm/mach-kirkwood/board-dt.c                  | 15 +------
> >  arch/arm/mm/cache-feroceon-l2.c                    | 46 ++++++++++++++++++++++
> >  5 files changed, 73 insertions(+), 14 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/arm/mrvl/foroceon.txt
> 
> for the next revision, please split out the dtsi changes into a separate
> patch.
> 
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/mrvl/foroceon.txt b/Documentation/devicetree/bindings/arm/mrvl/foroceon.txt
> > new file mode 100644
> > index 000000000000..8058676d1476
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/mrvl/foroceon.txt
> 
> name typo.
> 
> > @@ -0,0 +1,19 @@
> > +* Marvell Feroceon Cache
> > +
> > +Required properties:
> > +- compatible : Should be "marvell,feroceon-kirkwood".
> 
> This is a little ambiguous, I'd like to see 'l2' in the compatible string.

I will take Jason's advice on naming and change this..

> > +#ifdef CONFIG_OF
> > +static const struct of_device_id feroceon_ids[] __initconst = {
> > +	{ .compatible = "marvell,feroceon-kirkwood"},
> > +	{}
> > +};
> > +
> > +int __init feroceon_of_init(void)
> > +{
> > +	struct device_node *node;
> > +	void __iomem *base;
> > +	bool writethrough = false;
> > +	struct resource res;
> > +
> > +	node = of_find_matching_node(NULL, feroceon_ids);
> > +	if (!node) {
> > +		pr_info("Didn't find marvell,feroceon-*, not enabling it\n");
> > +		return -ENODEV;
> > +	}
> 
> I'd prefer to fallback to hardcoded register address here.  We know
> we're on kirkwood at this point.

We could also be on mv78xx0, sometime in the future. So we need to at
least look at the compatible string to see what SoC we are. It is also
rather ugly having hard coded addresses. We might as well go back to
platform devices.

I would prefer upping this to pr_err(FW_INFO, and not falling back to
hard coded values. This is not a fatal error, the machine continues to
run, just a bit slower.

     Andrew

^ permalink raw reply

* [PATCH v3 03/15] at91: dt: sam9261: Basic Device Tree support for the at91sam9261ek
From: Jean-Jacques Hiblot @ 2014-02-07  9:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207083007.GW9558@ns203013.ovh.net>

2014-02-07 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 16:57 Thu 23 Jan     , Jean-Jacques Hiblot wrote:
>> This patch implements a simple DTS to boot a at91sam9261ek with a dt-enabled
>> kernel (at91_dt_defconfig).
>> Only dbgu, nand and watchdog are described in the DT.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> ---
>>  arch/arm/boot/dts/Makefile          |   2 +
>>  arch/arm/boot/dts/at91sam9261ek.dts | 112 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 114 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/at91sam9261ek.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 772a30e..ece523d 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -11,6 +11,8 @@ dtb-$(CONFIG_ARCH_AT91) += ethernut5.dtb
>>  dtb-$(CONFIG_ARCH_AT91) += evk-pro3.dtb
>>  dtb-$(CONFIG_ARCH_AT91) += tny_a9260.dtb
>>  dtb-$(CONFIG_ARCH_AT91) += usb_a9260.dtb
>> +# sam9261
>> +dtb-$(CONFIG_ARCH_AT91) += at91sam9261ek.dtb
>>  # sam9263
>>  dtb-$(CONFIG_ARCH_AT91) += at91sam9263ek.dtb
>>  dtb-$(CONFIG_ARCH_AT91) += tny_a9263.dtb
>> diff --git a/arch/arm/boot/dts/at91sam9261ek.dts b/arch/arm/boot/dts/at91sam9261ek.dts
>> new file mode 100644
>> index 0000000..8909217
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/at91sam9261ek.dts
>> @@ -0,0 +1,112 @@
>> +/*
>> + * at91sam9261ek.dts - Device Tree file for Atmel at91sam9261 reference board
>> + *
>> + *  Copyright (C) 2013 Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> + *
>> + * Licensed under GPLv2 only.
>> + */
>> +/dts-v1/;
>> +#include "at91sam9261.dtsi"
>> +
>> +/ {
>> +     model = "Atmel at91sam9261ek";
>> +     compatible = "atmel,at91sam9261ek", "atmel,at91sam9261", "atmel,at91sam9";
>> +
>> +     chosen {
>> +             bootargs = "mem=64M console=ttyS0,115200";
> drop the mem=64M
my bad. It should indeed have been removed already.
>
> and use linux,stdout for the console
>> +     };
>> +
>> +     memory {
>> +             reg = <0x20000000 0x4000000>;
>> +     };
>> +
>> +     clocks {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             ranges;
>> +
>> +             main_clock: clock at 0 {
>> +                     compatible = "atmel,osc", "fixed-clock";
>> +                     clock-frequency = <18432000>;
>> +             };
>> +     };
>> +
>> +     ahb {
>> +
>> +             nand0: nand at 40000000 {
>> +                     nand-bus-width = <8>;
>> +                     nand-ecc-mode = "soft";
>> +                     nand-on-flash-bbt = <1>;
> it's a boolean just
copy paste from 9263ek DT.
I'll fix it there too.
>                         nand-on-flash-bbt;
>> +                     status = "okay";
>> +
>> +                     at91bootstrap at 0 {
>> +                             label = "at91bootstrap";
>> +                             reg = <0x0 0x40000>;
>> +                     };
>> +
>> +                     bootloader at 40000 {
>> +                             label = "bootloader";
>> +                             reg = <0x40000 0x80000>;
>> +                     };
>> +
>> +                     bootloaderenv at c0000 {
>> +                             label = "bootloader env";
>> +                             reg = <0xc0000 0xc0000>;
>> +                     };
>> +
>> +                     dtb at 180000 {
>> +                             label = "device tree";
>> +                             reg = <0x180000 0x80000>;
>> +                     };
>> +
>> +                     kernel at 200000 {
>> +                             label = "kernel";
>> +                             reg = <0x200000 0x600000>;
>> +                     };
>> +
>> +                     rootfs at 800000 {
>> +                             label = "rootfs";
>> +                             reg = <0x800000 0x0f800000>;
>> +                     };
>
> we really need to switch to UBI
This mapping is fine for UBI (in terms of alignment).
I could add UBI in the default bootargs
>> +             };
>> +
>> +             apb {
>> +                     dbgu: serial at fffff200 {
>> +                             status = "okay";
>> +                     };
>> +
>> +                     pinctrl at fffff400 {
>> +                             leds {
>> +                                     pinctrl_leds: leds-0 {
>> +                                             atmel,pins = <AT91_PIOA 13  AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> +                                             AT91_PIOA 14  AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>> +                                             AT91_PIOA 23  AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
>> +                                     };
>> +                             };
>> +                     };
>
> no need this drop it
>
> you need to only describe the GPIO if you have specific pinctrl property such
> as pull up & co
>> +
>> +                     watchdog at fffffd40 {
>> +                             status = "okay";
>> +                     };
>> +             };
>> +     };
>> +
>> +     leds {
>> +             compatible = "gpio-leds";
>> +             ds8 {
>> +                     label = "ds8";
>> +                     gpios = <&pioA 13 GPIO_ACTIVE_LOW>;
>> +                     linux,default-trigger = "none";
>> +             };
>> +             ds7 {
>> +                     label = "ds7";
>> +                     gpios = <&pioA 14 GPIO_ACTIVE_LOW>;
>> +                     linux,default-trigger = "nand-disk";
>> +             };
>> +             ds1 {
>> +                     label = "ds1";
>> +                     gpios = <&pioA 23 GPIO_ACTIVE_LOW>;
>> +                     linux,default-trigger = "heartbeat";
>> +             };
>> +     };
>> +};
>> --
>> 1.8.5.2
>>

^ permalink raw reply

* [PATCH v3 00/15] Device Tree support for the at91sam9261ek
From: Jean-Jacques Hiblot @ 2014-02-07  9:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207083304.GX9558@ns203013.ovh.net>

2014-02-07 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> Hi,
>
>         please re-organise your patch serie
>
>         first SoC patch
>           - first CCF
>           - one patch for the SoC
>         second add the board 1 patch
>
>         no need 15 patches
>
> Best Regards,
> J.
I don't know about this. It's always easier for the review and bug
chasing to keep the patch separated. It would make my task easier
though.
I'll wait for the feedback of Nicolas on this.

> On 16:57 Thu 23 Jan     , Jean-Jacques Hiblot wrote:
>> This patch set aims at bringing a basic device tree support for the sam9261.
>> It's mostly based on the sam9263 and sam9x5 stuff.
>>
>> Changes since V2:
>> * removed the smc driver from the serie. It'll be posted in an independent
>>   serie later.
>> * removed the DM9000 support (it'll come with the smc driver)
>> * the sam9261 now supports the Common Clock Framework (CCF). Note: to enable
>>   the CCF you must remove from the kernel config the platforms that don't
>>   support it.
>> * Added basic DT binding for the bus matrix
>> * Added support for USB host
>> * Added support for USB gadget
>> * in dts(i), replaced interrupt-parent with interrupts-extended
>> * changed the nand partition plan (same as for the sama5d3)
>> * removed 'mem' parameter in command line
>> * re-ordered dt-nodes in ascending address order.
>> * split the lcd support patch in 2 parts (SOC and board)
>>
>>
>> Change since V1:
>> * changed the DT representation to use address translation and separate the
>>   timings' configuration from the device properties by adding a "simple-bus"
>>   inetrmediate node.
>> * moved the smc driver from drivers/bus to drivers/memmory
>> * smc driver now accepts timings in nanoseconds as well as raw register values
>> * smc driver can clip the timings if they're out of bound and dump them to the
>>   console
>> * DM9000 timings are now described in nanosecs (for the virtue of example)
>>
>> supported features:
>> * dbgu
>> * nand
>> * lcd
>> * leds
>> * user buttons
>> * usb host
>> * usb gadget
>>
>> This serie relies on the 3 following patchs:
>> usb: ohci-at91: fix irq and iomem resource retrieval
>> usb: at91-udc: fix irq and iomem resource retrieval
>> ARM: at91: prepare sam9 dt boards transition to common
>>
>> Jean-Jacques
>>
>>
>>
>> Jean-Jacques Hiblot (15):
>>   at91: dt: Add at91sam9261 dt SoC support
>>   at91: dt: defconfig: Added the sam9261 to the list of supported SOCs
>>   at91: dt: sam9261: Basic Device Tree support for the at91sam9261ek
>>   ARM: at91: prepare common clk transition for sam9261 SoC
>>   ARM: at91: move sam9261 SoC to common clk
>>   at91: dt: at91sam9261ek: Adds DT entries for the 4 user buttons
>>   at91: dt: sam9261: Added the descriptions of hck0 and hck1 clocks
>>     (CCF)
>>   at91: dt: sam9261: Added hclk declaration for the fb driver (non-CCF)
>>   at91: dt: sam9261: Added support for the LCD display
>>   at91: dt: at91sam9261ek: Added support for the LCD display
>>   at91: dt: Adds support for the bus matrix declaration in the device
>>     tree
>>   at91: dt: sam9261: adds description for the bus matrix
>>   at91: dt: sam9261: CCF: Added USB clocks
>>   at91: dt: at91sam9261ek: Enabled the USB host port (OHCI)
>>   at91: dt: at91sam9261ek: Enabled the USB device port
>>
>>  arch/arm/boot/dts/Makefile          |   2 +
>>  arch/arm/boot/dts/at91sam9261.dtsi  | 724 ++++++++++++++++++++++++++++++++++++
>>  arch/arm/boot/dts/at91sam9261ek.dts | 191 ++++++++++
>>  arch/arm/configs/at91_dt_defconfig  |   1 +
>>  arch/arm/mach-at91/Kconfig          |   1 -
>>  arch/arm/mach-at91/at91sam9261.c    |  24 +-
>>  arch/arm/mach-at91/setup.c          |  23 ++
>>  7 files changed, 963 insertions(+), 3 deletions(-)
>>  create mode 100644 arch/arm/boot/dts/at91sam9261.dtsi
>>  create mode 100644 arch/arm/boot/dts/at91sam9261ek.dts
>>
>> --
>> 1.8.5.2
>>

^ permalink raw reply

* [PATCH 19/21] ARM: MVEBU: Simplifiy headers and make local
From: Thomas Petazzoni @ 2014-02-07  9:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207092047.GD17758@lunn.ch>

Dear Andrew Lunn,

On Fri, 7 Feb 2014 10:20:47 +0100, Andrew Lunn wrote:

> I wanted to keep 14/21 as a simple move of files, doing the minimal
> needed to make it boot. The patches that follow then clean up. I found
> that simpler to follow what was going on, since there is nothing
> hidden in the move patch.
> 
> I can however squash these together if that is what people want to
> see.

Ok. Then maybe the commit log should indicate this (that the commit is
only a move and that followup commits clean things up).

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-07  9:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8b4dad82-c72a-4e1f-b1af-b8c7964bbf24@TX2EHSMHS043.ehs.local>

On 02/07/2014 09:42 AM, Appana Durga Kedareswara Rao wrote:
>>> ---
>>> This patch is rebased on the 3.14 rc1 kernel.
>>> ---
>>>  .../devicetree/bindings/net/can/xilinx_can.txt     |   43 +
>>>  drivers/net/can/Kconfig                            |    8 +
>>>  drivers/net/can/Makefile                           |    1 +
>>>  drivers/net/can/xilinx_can.c                       | 1150 ++++++++++++++++++++
>>>  4 files changed, 1202 insertions(+), 0 deletions(-)  create mode
>>> 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>>  create mode 100644 drivers/net/can/xilinx_can.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> new file mode 100644
>>> index 0000000..34f9643
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> @@ -0,0 +1,43 @@
>>> +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings
>>> +---------------------------------------------------------
>>> +
>>> +Required properties:
>>> +- compatible               : Should be "xlnx,zynq-can-1.00.a" for Zynq
>> CAN
>>> +                     controllers and "xlnx,axi-can-1.00.a" for Axi CAN
>>> +                     controllers.
>>> +- reg                      : Physical base address and size of the Axi CAN/Zynq
>>> +                     CANPS registers map.
>>> +- interrupts               : Property with a value describing the interrupt
>>> +                     number.
>>> +- interrupt-parent : Must be core interrupt controller
>>> +- clock-names              : List of input clock names - "ref_clk",
>> "aper_clk"
>>> +                     (See clock bindings for details. Two clocks are
>>> +                      required for Zynq CAN. For Axi CAN
>>> +                      case it is one(ref_clk)).
>>> +- clocks           : Clock phandles (see clock bindings for details).
>>> +- xlnx,can-tx-dpth : Can Tx fifo depth (Required for Axi CAN).
>>> +- xlnx,can-rx-dpth : Can Rx fifo depth (Required for Axi CAN).
>>> +
>>> +
>>> +Example:
>>> +
>>> +For Zynq CANPS Dts file:
>>> +   zynq_can_0: zynq-can at e0008000 {
>>> +                   compatible = "xlnx,zynq-can-1.00.a";
>>> +                   clocks = <&clkc 19>, <&clkc 36>;
>>> +                   clock-names = "ref_clk", "aper_clk";
>>> +                   reg = <0xe0008000 0x1000>;
>>> +                   interrupts = <0 28 4>;
>>> +                   interrupt-parent = <&intc>;
>>
>> Above xlnx,can-{rx,tx}-dpth is mentioned as required, but it's not in the
>> Zynq example.
> 
> One of the Difference b/w the AXI CAN and zynq CAN is in AXI CAN the fifo depth(tx,rx)
> Is user configurable. But in case of ZYNQ CAN  the fifo depth  is fixed for tx and rx fifo's(64)
> Xlnx,can-{rx,tx}-dpth is required only for AXI CAN case it is not required for zynq CAN.
> That's why didn't putted that property in device tree.

The device tree should be a hardware only description and should not
hold any user configurable data. Please split your patch into two
patches. The first one should add the driver with a fixed fifo size
(e.g. 0x40) for the AXI, too. The second patch should make the fifo
configurable via device tree.

If it's acceptable to describe the fifo usage by device tree, I'd like
to make it a generic CAN driver option. But we have to look around, e.g.
what the Ethernet driver use to configure their hardware.

> 
>>
>>> +           };
>>> +For Axi CAN Dts file:
>>> +   axi_can_0: axi-can at 40000000 {
>>> +                   compatible = "xlnx,axi-can-1.00.a";
>>> +                   clocks = <&clkc 0>;
>>> +                   clock-names = "ref_clk" ;
>>> +                   reg = <0x40000000 0x10000>;
>>> +                   interrupt-parent = <&intc>;
>>> +                   interrupts = <0 59 1>;
>>> +                   xlnx,can-tx-dpth = <0x40>;
>>> +                   xlnx,can-rx-dpth = <0x40>;
>>> +           };
>>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
>>> d447b88..2344fb5 100644
>>> --- a/drivers/net/can/Kconfig
>>> +++ b/drivers/net/can/Kconfig
>>> @@ -125,6 +125,14 @@ config CAN_GRCAN
>>>       endian syntheses of the cores would need some modifications on
>>>       the hardware level to work.
>>>
>>> +config CAN_XILINXCAN
>>> +   tristate "Xilinx CAN"
>>> +   depends on ARCH_ZYNQ || MICROBLAZE
>>> +   default n
>>
>> "default n" is default, please remove.
>>
> 
> Ok
> 
>>> +   ---help---
>>> +     Xilinx CAN driver. This driver supports both soft AXI CAN IP and
>>> +     Zynq CANPS IP.
>>> +
>>>  source "drivers/net/can/mscan/Kconfig"
>>>
>>>  source "drivers/net/can/sja1000/Kconfig"
>>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index
>>> c744039..0b8e11e 100644
>>> --- a/drivers/net/can/Makefile
>>> +++ b/drivers/net/can/Makefile
>>> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)      += janz-
>> ican3.o
>>>  obj-$(CONFIG_CAN_FLEXCAN)  += flexcan.o
>>>  obj-$(CONFIG_PCH_CAN)              += pch_can.o
>>>  obj-$(CONFIG_CAN_GRCAN)            += grcan.o
>>> +obj-$(CONFIG_CAN_XILINXCAN)        += xilinx_can.o
>>>
>>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git
>>> a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c new file
>>> mode 100644 index 0000000..c1b2b9d
>>> --- /dev/null
>>> +++ b/drivers/net/can/xilinx_can.c
>>> @@ -0,0 +1,1150 @@
>>> +/* Xilinx CAN device driver
>>> + *
>>> + * Copyright (C) 2012 - 2014 Xilinx, Inc.
>>> + * Copyright (C) 2009 PetaLogix. All rights reserved.
>>> + *
>>> + * Description:
>>> + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
>>> + * This program is free software: you can redistribute it and/or
>>> +modify
>>> + * it under the terms of the GNU General Public License as published
>>> +by
>>> + * the Free Software Foundation, either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/init.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/netdevice.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/skbuff.h>
>>> +#include <linux/string.h>
>>> +#include <linux/types.h>
>>> +#include <linux/can/dev.h>
>>> +#include <linux/can/error.h>
>>> +#include <linux/can/led.h>
>>> +
>>> +#define DRIVER_NAME        "XILINX_CAN"
>>> +
>>> +/* CAN registers set */
>>> +#define XCAN_SRR_OFFSET                    0x00 /* Software reset */
>>> +#define XCAN_MSR_OFFSET                    0x04 /* Mode select */
>>> +#define XCAN_BRPR_OFFSET           0x08 /* Baud rate prescaler */
>>> +#define XCAN_BTR_OFFSET                    0x0C /* Bit timing */
>>> +#define XCAN_ECR_OFFSET                    0x10 /* Error counter */
>>> +#define XCAN_ESR_OFFSET                    0x14 /* Error status */
>>> +#define XCAN_SR_OFFSET                     0x18 /* Status */
>>> +#define XCAN_ISR_OFFSET                    0x1C /* Interrupt status */
>>> +#define XCAN_IER_OFFSET                    0x20 /* Interrupt enable */
>>> +#define XCAN_ICR_OFFSET                    0x24 /* Interrupt clear */
>>> +#define XCAN_TXFIFO_ID_OFFSET              0x30 /* TX FIFO ID */
>>> +#define XCAN_TXFIFO_DLC_OFFSET             0x34 /* TX FIFO DLC */
>>> +#define XCAN_TXFIFO_DW1_OFFSET             0x38 /* TX FIFO Data
>> Word 1 */
>>> +#define XCAN_TXFIFO_DW2_OFFSET             0x3C /* TX FIFO Data
>> Word 2 */
>>> +#define XCAN_RXFIFO_ID_OFFSET              0x50 /* RX FIFO ID */
>>> +#define XCAN_RXFIFO_DLC_OFFSET             0x54 /* RX FIFO DLC */
>>> +#define XCAN_RXFIFO_DW1_OFFSET             0x58 /* RX FIFO Data
>> Word 1 */
>>> +#define XCAN_RXFIFO_DW2_OFFSET             0x5C /* RX FIFO Data
>> Word 2 */
>>> +
>>> +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
>>> +#define XCAN_SRR_CEN_MASK          0x00000002 /* CAN enable */
>>> +#define XCAN_SRR_RESET_MASK                0x00000001 /* Soft Reset the
>> CAN core */
>>> +#define XCAN_MSR_LBACK_MASK                0x00000002 /* Loop back
>> mode select */
>>> +#define XCAN_MSR_SLEEP_MASK                0x00000001 /* Sleep mode
>> select */
>>> +#define XCAN_BRPR_BRP_MASK         0x000000FF /* Baud rate
>> prescaler */
>>> +#define XCAN_BTR_SJW_MASK          0x00000180 /* Synchronous
>> jump width */
>>> +#define XCAN_BTR_TS2_MASK          0x00000070 /* Time segment
>> 2 */
>>> +#define XCAN_BTR_TS1_MASK          0x0000000F /* Time segment
>> 1 */
>>> +#define XCAN_ECR_REC_MASK          0x0000FF00 /* Receive error
>> counter */
>>> +#define XCAN_ECR_TEC_MASK          0x000000FF /* Transmit error
>> counter */
>>> +#define XCAN_ESR_ACKER_MASK                0x00000010 /* ACK error */
>>> +#define XCAN_ESR_BERR_MASK         0x00000008 /* Bit error */
>>> +#define XCAN_ESR_STER_MASK         0x00000004 /* Stuff error */
>>> +#define XCAN_ESR_FMER_MASK         0x00000002 /* Form error */
>>> +#define XCAN_ESR_CRCER_MASK                0x00000001 /* CRC error */
>>> +#define XCAN_SR_TXFLL_MASK         0x00000400 /* TX FIFO is full
>> */
>>> +#define XCAN_SR_ESTAT_MASK         0x00000180 /* Error status */
>>> +#define XCAN_SR_ERRWRN_MASK                0x00000040 /* Error warning
>> */
>>> +#define XCAN_SR_NORMAL_MASK                0x00000008 /* Normal mode
>> */
>>> +#define XCAN_SR_LBACK_MASK         0x00000002 /* Loop back
>> mode */
>>> +#define XCAN_SR_CONFIG_MASK                0x00000001 /* Configuration
>> mode */
>>> +#define XCAN_IXR_TXFEMP_MASK               0x00004000 /* TX FIFO Empty
>> */
>>> +#define XCAN_IXR_WKUP_MASK         0x00000800 /* Wake up
>> interrupt */
>>> +#define XCAN_IXR_SLP_MASK          0x00000400 /* Sleep
>> interrupt */
>>> +#define XCAN_IXR_BSOFF_MASK                0x00000200 /* Bus off
>> interrupt */
>>> +#define XCAN_IXR_ERROR_MASK                0x00000100 /* Error interrupt
>> */
>>> +#define XCAN_IXR_RXNEMP_MASK               0x00000080 /* RX FIFO
>> NotEmpty intr */
>>> +#define XCAN_IXR_RXOFLW_MASK               0x00000040 /* RX FIFO
>> Overflow intr */
>>> +#define XCAN_IXR_RXOK_MASK         0x00000010 /* Message
>> received intr */
>>> +#define XCAN_IXR_TXOK_MASK         0x00000002 /* TX successful
>> intr */
>>> +#define XCAN_IXR_ARBLST_MASK               0x00000001 /* Arbitration
>> lost intr */
>>> +#define XCAN_IDR_ID1_MASK          0xFFE00000 /* Standard msg
>> identifier */
>>> +#define XCAN_IDR_SRR_MASK          0x00100000 /* Substitute
>> remote TXreq */
>>> +#define XCAN_IDR_IDE_MASK          0x00080000 /* Identifier
>> extension */
>>> +#define XCAN_IDR_ID2_MASK          0x0007FFFE /* Extended
>> message ident */
>>> +#define XCAN_IDR_RTR_MASK          0x00000001 /* Remote TX
>> request */
>>> +#define XCAN_DLCR_DLC_MASK         0xF0000000 /* Data length
>> code */
>>> +
>>> +#define XCAN_INTR_ALL              (XCAN_IXR_TXOK_MASK |
>> XCAN_IXR_BSOFF_MASK |\
>>> +                            XCAN_IXR_WKUP_MASK |
>> XCAN_IXR_SLP_MASK | \
>>> +                            XCAN_IXR_RXNEMP_MASK |
>> XCAN_IXR_ERROR_MASK | \
>>> +                            XCAN_IXR_ARBLST_MASK |
>> XCAN_IXR_RXOK_MASK)
>>> +
>>> +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
>>> +#define XCAN_BTR_SJW_SHIFT         7  /* Synchronous jump width
>> */
>>> +#define XCAN_BTR_TS2_SHIFT         4  /* Time segment 2 */
>>> +#define XCAN_IDR_ID1_SHIFT         21 /* Standard Messg
>> Identifier */
>>> +#define XCAN_IDR_ID2_SHIFT         1  /* Extended Message
>> Identifier */
>>> +#define XCAN_DLCR_DLC_SHIFT                28 /* Data length code */
>>> +#define XCAN_ESR_REC_SHIFT         8  /* Rx Error Count */
>>> +
>>> +/* CAN frame length constants */
>>> +#define XCAN_ECHO_SKB_MAX          64
>>> +#define XCAN_NAPI_WEIGHT           64
>>> +#define XCAN_FRAME_MAX_DATA_LEN            8
>>> +#define XCAN_TIMEOUT                       (50 * HZ)
>>> +
>>> +/**
>>> + * struct xcan_priv - This definition define CAN driver instance
>>> + * @can:                   CAN private data structure.
>>> + * @open_time:                     For holding timeout values
>>> + * @waiting_ech_skb_index: Pointer for skb
>>> + * @ech_skb_next:          This tell the next packet in the queue
>>> + * @waiting_ech_skb_num:   Gives the number of packets waiting
>>> + * @xcan_echo_skb_max_tx:  Maximum number packets the driver
>> can send
>>> + * @xcan_echo_skb_max_rx:  Maximum number packets the driver
>> can receive
>>> + * @napi:                  NAPI structure
>>> + * @ech_skb_lock:          For spinlock purpose
>>> + * @read_reg:                      For reading data from CAN registers
>>> + * @write_reg:                     For writing data to CAN registers
>>> + * @dev:                   Network device data structure
>>> + * @reg_base:                      Ioremapped address to registers
>>> + * @irq_flags:                     For request_irq()
>>> + * @aperclk:                       Pointer to struct clk
>>> + * @devclk:                        Pointer to struct clk
>>> + */
>>> +struct xcan_priv {
>>> +   struct can_priv can;
>>> +   int open_time;
>>> +   int waiting_ech_skb_index;
>>> +   int ech_skb_next;
>>> +   int waiting_ech_skb_num;
>>> +   int xcan_echo_skb_max_tx;
>>> +   int xcan_echo_skb_max_rx;
>>> +   struct napi_struct napi;
>>> +   spinlock_t ech_skb_lock;
>>> +   u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>>> +   void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>>
>> Why do you have {read,write}_reg function here?
> 
> xcan_write_reg/xcan_read_reg is used because this IP is also in big endian version.
> We won't support It but if someone else want to support that's why putted these function's here.
> 
> If you are not ok with this will remove.

As far as I know, the kernel will either be little or big endian, so we
can make a use of an ifdef for here, if we support LE and BE.

> 
>>
>>> +   struct net_device *dev;
>>> +   void __iomem *reg_base;
>>> +   unsigned long irq_flags;
>>> +   struct clk *aperclk;
>>> +   struct clk *devclk;
>>> +};
>>> +
>>> +/* CAN Bittiming constants as per Xilinx CAN specs */ static struct
>>> +can_bittiming_const xcan_bittiming_const = {

Please make it "static const struct"

>>> +   .name = DRIVER_NAME,
>>> +   .tseg1_min = 1,
>>> +   .tseg1_max = 16,
>>> +   .tseg2_min = 1,
>>> +   .tseg2_max = 8,
>>> +   .sjw_max = 4,
>>> +   .brp_min = 1,
>>> +   .brp_max = 256,
>>> +   .brp_inc = 1,
>>> +};
>>> +
>>> +/**
>>> + * xcan_write_reg - Write a value to the device register
>>> + * @priv:  Driver private data structure
>>> + * @reg:   Register offset
>>> + * @val:   Value to write at the Register offset
>>> + *
>>> + * Write data to the paricular CAN register  */ static void
>>> +xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val) {
>>> +   writel(val, priv->reg_base + reg);
>>> +}
>>> +
>>> +/**
>>> + * xcan_read_reg - Read a value from the device register
>>> + * @priv:  Driver private data structure
>>> + * @reg:   Register offset
>>> + *
>>> + * Read data from the particular CAN register
>>> + * Return: value read from the CAN register  */ static u32
>>> +xcan_read_reg(const struct xcan_priv *priv, int reg) {
>>> +   return readl(priv->reg_base + reg);
>>> +}
>>> +
>>> +/**
>>> + * set_reset_mode - Resets the CAN device mode
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This is the driver reset mode routine.The driver
>>> + * enters into configuration mode.
>>> + *
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +set_reset_mode(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   unsigned long timeout;
>>> +
>>> +   priv->can.state = CAN_STATE_STOPPED;
>>> +   priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_OFFSET);
>>> +
>>> +   timeout = jiffies + XCAN_TIMEOUT;
>>> +   while (!(priv->read_reg(priv, XCAN_SR_OFFSET) &
>> XCAN_SR_CONFIG_MASK)) {
>>> +           if (time_after(jiffies, timeout)) {
>>> +                   netdev_warn(ndev, "timedout waiting for config
>> mode\n");
>>> +                   return -ETIMEDOUT;
>>> +           }
>>> +           schedule_timeout(1);
>>
>> better use usleep_range()
>>
> 
> Ok
>>> +   }
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_set_bittiming - CAN set bit timing routine
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This is the driver set bittiming  routine.
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_set_bittiming(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   struct can_bittiming *bt = &priv->can.bittiming;
>>> +   u32 btr0, btr1;
>>> +   u32 is_config_mode;
>>> +
>>> +   /* Check whether Xilinx CAN is in configuration mode.
>>> +    * It cannot set bit timing if Xilinx CAN is not in configuration mode.
>>> +    */
>>> +   is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) &
>>> +                           XCAN_SR_CONFIG_MASK;
>>> +   if (!is_config_mode) {
>>> +           netdev_alert(ndev,
>>> +                   "Cannot set bittiming can is not in config mode\n");
>>> +           return -EPERM;
>>> +   }
>>> +
>>> +   netdev_dbg(ndev,
>> "brp=%d,prop=%d,phase_seg1:%d,phase_reg2=%d,sjw=%d\n",
>>> +                   bt->brp, bt->prop_seg, bt->phase_seg1, bt-
>>> phase_seg2,
>>> +                   bt->sjw);
>>
>> I think this dbg can be removed, as it just prints the userspace values.
>>
> Ok
> 
>>> +
>>> +   /* Setting Baud Rate prescalar value in BRPR Register */
>>> +   btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK;
>>> +
>>> +   /* Setting Time Segment 1 in BTR Register */
>>> +   btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK;
>>> +
>>> +   /* Setting Time Segment 2 in BTR Register */
>>> +   btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) &
>>> +           XCAN_BTR_TS2_MASK;
>>> +
>>> +   /* Setting Synchronous jump width in BTR Register */
>>> +   btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) &
>> XCAN_BTR_SJW_MASK;
>>> +
>>> +   if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>>> +           netdev_info(ndev, "Doesn't support Triple Sampling\n");
>>
>> no need to check, it's not passed to the driver until you advertise you
>> support it (see priv->can.ctrlmode_supported).
> 
> 
> Ok
> 
>>
>>> +   netdev_dbg(ndev, "Setting BTR0=0x%02x BTR1=0x%02x\n", btr0,
>> btr1);
>>> +
>>> +   priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0);
>>> +   priv->write_reg(priv, XCAN_BTR_OFFSET, btr1);
>>> +
>>> +   netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
>>> +                   priv->read_reg(priv, XCAN_BRPR_OFFSET),
>>> +                   priv->read_reg(priv, XCAN_BTR_OFFSET));
>>
>> One of the dbg should be enough.
>>
> 
> 
> Ok
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_start - This the drivers start routine
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This is the drivers start routine.
>>> + * Based on the State of the CAN device it puts
>>> + * the CAN device into a proper mode.
>>> + *
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_start(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> +   /* Check if it is in reset mode */
>>> +   if (priv->can.state != CAN_STATE_STOPPED)
>>> +           set_reset_mode(ndev);
>>
>> Please check return value of set_reset_mode
> 
> Ok
> 
> 
>>
>>> +
>>> +   /* Enable interrupts */
>>> +   priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL);
>>> +
>>> +   /* Check whether it is loopback mode or normal mode  */
>>> +   if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> +           /* Put device into loopback mode */
>>> +           priv->write_reg(priv, XCAN_MSR_OFFSET,
>> XCAN_MSR_LBACK_MASK);
>>> +   else
>>> +           /* The device is in normal mode */
>>> +           priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> +
>>> +   if (priv->can.state == CAN_STATE_STOPPED) {
>>
>> I think your driver is always in CAN_STATE_STOPPED, right?
> 
> Usually it in stopped state only(configuration mode) during initialization.
> 
>>
>>> +           /* Enable Xilinx CAN */
>>> +           priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_CEN_MASK);
>>> +           priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +           if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
>>> +                   while ((priv->read_reg(priv, XCAN_SR_OFFSET) &
>>> +                                   XCAN_SR_LBACK_MASK) == 0)
>>> +                                   ;
>>> +           } else {
>>> +                   while ((priv->read_reg(priv, XCAN_SR_OFFSET)
>>> +                                   & XCAN_SR_NORMAL_MASK) == 0)
>>> +                                   ;
>>
>> Please add a timeout to the loops.
> 
> Ok
>>
>>> +           }
>>> +           netdev_dbg(ndev, "status:#x%08x\n",
>>> +                           priv->read_reg(priv, XCAN_SR_OFFSET));
>>> +   }
>>> +   priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_do_set_mode - This sets the mode of the driver
>>> + * @ndev:  Pointer to net_device structure
>>> + * @mode:  Tells the mode of the driver
>>> + *
>>> + * This check the drivers state and calls the
>>> + * the corresponding modes to set.
>>> + *
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_do_set_mode(struct net_device *ndev, enum can_mode mode) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   int ret;
>>> +
>>> +   netdev_dbg(ndev, "Setting the mode of the driver%s\n", __func__);
>>
>> please remove dbg
> 
> Ok
>>
>>> +
>>> +   if (!priv->open_time)
>>> +           return -EINVAL;
>>
>> please remove open_time completely.
> 
> Ok
> 
> 
> 
>>
>>> +
>>> +   switch (mode) {
>>> +   case CAN_MODE_START:
>>> +           ret = xcan_start(ndev);
>>> +           if (ret < 0)
>>> +                   netdev_err(ndev, "xcan_start failed!\n");
>>> +
>>> +           if (netif_queue_stopped(ndev))
>>> +                   netif_wake_queue(ndev);
>>
>> just call wake_queue
> 
> Ok. But existing drivers are using netif_wake_queue right?

Sorry, I meant: Remove the if, just call netif_wake_queue directly.

> 
>>> +           break;
>>> +   default:
>>> +           ret = -EOPNOTSUPP;
>>> +           break;
>>> +   }
>>> +
>>> +   return ret;
>>> +}
>>> +
>>> +/**
>>> + * xcan_start_xmit - Starts the transmission
>>> + * @skb:   sk_buff pointer that contains data to be Txed
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This function is invoked from upper layers to initiate
>>> +transmission. This
>>> + * function uses the next available free txbuff and populates their
>>> +fields to
>>> + * start the transmission.
>>> + *
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   struct net_device_stats *stats = &ndev->stats;
>>> +   struct can_frame *cf = (struct can_frame *)skb->data;
>>> +   u32 id, dlc, tmp_dw1, tmp_dw2 = 0, data1, data2 = 0;
>>> +   unsigned long flags;
>>> +
>>
>> Please add here:
>>
>>       if (can_dropped_invalid_skb(dev, skb))
>>               return NETDEV_TX_OK;
> 
> Ok
> 
> 
> 
>>
>>> +   /* Check if the TX buffer is full */
>>> +   if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK)
>> {
>>> +           netif_stop_queue(ndev);
>>> +           netdev_err(ndev, "TX register is still full!\n");
>>> +           return NETDEV_TX_BUSY;
>>> +   } else if (priv->waiting_ech_skb_num == priv-
>>> xcan_echo_skb_max_tx) {
>>> +           netif_stop_queue(ndev);
>>> +           netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n",
>>> +                   priv->waiting_ech_skb_num, priv-
>>> xcan_echo_skb_max_tx);
>>> +           return NETDEV_TX_BUSY;
>>> +   }
>>
>> You should handle flow control after you put the CAN frame into the
>> hardware, but before activating the TX complete interrutp.
>>
> 
> Ok
> 
> 
>>> +   /* Watch carefully on the bit sequence */
>>> +   if ((cf->can_id & CAN_EFF_FLAG) == 0) {
>>
>> Nitpick easier to read is:
>>
>>
>>       if (cf->can_id & CAN_EFF_FLAG) {
>>               /* EFF handling */
>>       } else {
>>               /* STD handling */
>>       }
>>
> Ok
> 
> 
>>> +           /* Standard CAN ID format */
>>> +           id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT)
>> &
>>> +                   XCAN_IDR_ID1_MASK;
>>> +
>>> +           if (cf->can_id & CAN_RTR_FLAG)
>>> +                   /* Extended frames remote TX request */
>>> +                   id |= XCAN_IDR_SRR_MASK;
>>> +   } else {
>>> +           /* Extended CAN ID format */
>>> +           id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT)
>> &
>>> +                   XCAN_IDR_ID2_MASK;
>>> +           id |= (((cf->can_id & CAN_EFF_MASK) >>
>>> +                   (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
>>> +                   XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
>>> +
>>> +           /* The substibute remote TX request bit should be "1"
>>> +            * for extended frames as in the Xilinx CAN datasheet
>>> +            */
>>> +           id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
>>> +
>>> +           if (cf->can_id & CAN_RTR_FLAG)
>>> +                   /* Extended frames remote TX request */
>>> +                   id |= XCAN_IDR_RTR_MASK;
>>> +   }
>>> +
>>> +   dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;
>>
>>       With the above check can_dlc is valid.
>>
>>> +
>>> +   tmp_dw1 = le32_to_cpup((u32 *)(cf->data));
>>> +   data1 = htonl(tmp_dw1);
>>
>> This looks broken. cf->data is in big endian, what is the endianess of your
>> registers?
>>
> 
> Endianess of  our registers is little endian that's why need to do like this.

Is it always little endian or native endianess (e.g. when we're running
a big endian kernel)?

As the driver support only little endian so far, make I suggest to use
be32_to_cpup():

	u32 data[2] = { 0, 0 };

	if (dlc > 0)
		data[0] = be32_to_cpup((__be32 *)(cf->data + 0));
	if (dlc > 4)
		data[1] = be32_to_cpup((__be32 *)(cf->data + 4));

> 
>>> +   if (dlc > 4) {
>>> +           tmp_dw2 = le32_to_cpup((u32 *)(cf->data + 4));
>>> +           data2 = htonl(tmp_dw2);
>>> +   }
>>> +
>>> +   netdev_dbg(ndev,
>> "tx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
>>> +                   id, dlc, data1, data2);
>>
>> please remove the dbg
>>
> 
> Ok
> 
>>> +   /* Write the Frame to Xilinx CAN TX FIFO */
>>> +   priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
>>> +   priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
>>> +   priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data1);
>>> +   priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2);
>>
>> Which write triggers the transmission?
>>
> 
> The last write triggers the transmission.
> If we are sending data less than 4 bytes in that case also we need to
> Fill this( XCAN_TXFIFO_DW2_OFFSET) with default value .

I see, can you please add a comment to the code. What about
XCAN_TXFIFO_DW1_OFFSET in case dlc == 0?

> 
>>> +   stats->tx_bytes += cf->can_dlc;
>>
>> Can you move the tx_bytes += to your tx-complete routine?
> 
> Ok
>>
>>> +   ndev->trans_start = jiffies;
>>
>> Please remove
> 
> Ok
> 
> 
>>> +
>>> +   can_put_echo_skb(skb, ndev, priv->ech_skb_next);
>>
>> This looks racy, first fill the echo_skb, then start the transmission.
> 
> Ok But I didn't understand it clearly.
> Will you please explain a little clear.

This will trigger transmission:

	priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2);

Then you get a tx-complete interrupt.
The tx-complete handler takes care about the echo skb. But the echo skb
hasn't been filed yet.
Your tx code continues:

	can_put_echo_skb(skb, ndev, priv->ech_skb_next);

The echo skb handling is now totlaly messed up :)

> 
> 
>>> +
>>> +   priv->ech_skb_next = (priv->ech_skb_next + 1) %
>>> +                                   priv->xcan_echo_skb_max_tx;
>>> +
>>> +   spin_lock_irqsave(&priv->ech_skb_lock, flags);
>>> +   priv->waiting_ech_skb_num++;
>>> +   spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>>> +
>>
>> Please move the flow controll handling here.
>>
> 
> Ok
> 
>>> +   return NETDEV_TX_OK;
>>> +}
>>> +
>>> +/**
>>> + * xcan_rx -  Is called from CAN isr to complete the received
>>> + *         frame  processing
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This function is invoked from the CAN isr(poll) to process the Rx
>>> +frames. It
>>> + * does minimal processing and invokes "netif_receive_skb" to
>>> +complete further
>>> + * processing.
>>> + * Return: 0 on success and negative error value on error  */ static
>>> +int xcan_rx(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   struct net_device_stats *stats = &ndev->stats;
>>> +   struct can_frame *cf;
>>> +   struct sk_buff *skb;
>>> +   u32 id_xcan, dlc, data1, data2;
>>> +
>>> +   skb = alloc_can_skb(ndev, &cf);
>>> +   if (!skb)
>>> +           return -ENOMEM;
>>> +
>>> +   /* Read a frame from Xilinx zynq CANPS */
>>> +   id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET);
>>> +   dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) &
>> XCAN_DLCR_DLC_MASK;
>>> +   data1 = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET);
>>> +   data2 = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET);
>>
>> If you don't use data? below, don't read them in the first place. Better move
>> the read below, where you fill the data of the can_frame.
>>
> 
> Ok
> 
>> +     netdev_dbg(ndev,
>> "rx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
>>> +           id_xcan, dlc, data1, data2);
>>>
>> please remove dbg
> 
> Ok
> 
>>
>>  +
>>> +   /* Change Xilinx CAN data length format to socketCAN data format
>> */
>>> +   cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
>>> +                           XCAN_DLCR_DLC_SHIFT);
>>> +
>>> +   /* Change Xilinx CAN ID format to socketCAN ID format */
>>> +   if (id_xcan & XCAN_IDR_IDE_MASK) {
>>> +           /* The received frame is an Extended format frame */
>>> +           cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
>>> +           cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
>>> +                           XCAN_IDR_ID2_SHIFT;
>>> +           cf->can_id |= CAN_EFF_FLAG;
>>> +           if (id_xcan & XCAN_IDR_RTR_MASK)
>>> +                   cf->can_id |= CAN_RTR_FLAG;
>>> +   } else {
>>> +           /* The received frame is a standard format frame */
>>> +           cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
>>> +                           XCAN_IDR_ID1_SHIFT;
>>> +           if (id_xcan & XCAN_IDR_RTR_MASK)
>>> +                   cf->can_id |= CAN_RTR_FLAG;
>>> +   }
>>> +
>>> +   /* Change Xilinx CAN data format to socketCAN data format */
>>
>> Don't fill cf->data if RTR is set. The endianess handling looks weird here,
>> too.
>>
> OK
> 
>>> +   *(u32 *)(cf->data) = ntohl(data1);
>>> +   if (cf->can_dlc > 4)
>>> +           *(u32 *)(cf->data + 4) = ntohl(data2);
>>> +   else
>>> +           *(u32 *)(cf->data + 4) = 0;
>>
>> no need to set to zero
>>
> Ok
>>> +   stats->rx_bytes += cf->can_dlc;
>>
>> please group rx_bytes and rx_packets handling
>>> +
> 
> Ok
>>> +   can_led_event(ndev, CAN_LED_EVENT_RX);
>>> +
>>> +   netif_receive_skb(skb);
>>> +
>>> +   stats->rx_packets++;
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_err_interrupt - error frame Isr
>>> + * @ndev:  net_device pointer
>>> + * @isr:   interrupt status register value
>>> + *
>>> + * This is the CAN error interrupt and it will
>>> + * check the the type of error and forward the error
>>> + * frame to upper layers.
>>> + */
>>> +static void xcan_err_interrupt(struct net_device *ndev, u32 isr) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   struct net_device_stats *stats = &ndev->stats;
>>> +   struct can_frame *cf;
>>> +   struct sk_buff *skb;
>>> +   u32 err_status, status;
>>> +
>>> +   skb = alloc_can_err_skb(ndev, &cf);
>>> +   if (!skb) {
>>> +           netdev_err(ndev, "alloc_can_err_skb() failed!\n");
>>> +           return;
>>> +   }
>>> +
>>> +   err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
>>> +   priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);
>>> +   status = priv->read_reg(priv, XCAN_SR_OFFSET);
>>> +
>>> +   if (isr & XCAN_IXR_BSOFF_MASK) {
>>> +           priv->can.state = CAN_STATE_BUS_OFF;
>>> +           cf->can_id |= CAN_ERR_BUSOFF;
>>> +           priv->can.can_stats.bus_off++;
>>> +           /* Leave device in Config Mode in bus-off state */
>>> +           priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_RESET_MASK);
>>> +           can_bus_off(ndev);
>>> +   } else if ((status & XCAN_SR_ESTAT_MASK) ==
>> XCAN_SR_ESTAT_MASK) {
>>> +           cf->can_id |= CAN_ERR_CRTL;
>>> +           priv->can.state = CAN_STATE_ERROR_PASSIVE;
>>> +           priv->can.can_stats.error_passive++;
>>> +           cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE |
>>> +                                   CAN_ERR_CRTL_TX_PASSIVE;
>>> +   } else if (status & XCAN_SR_ERRWRN_MASK) {
>>> +           cf->can_id |= CAN_ERR_CRTL;
>>> +           priv->can.state = CAN_STATE_ERROR_WARNING;
>>> +           priv->can.can_stats.error_warning++;
>>> +           cf->data[1] |= CAN_ERR_CRTL_RX_WARNING |
>>> +                                   CAN_ERR_CRTL_TX_WARNING;
>>> +   }
>>> +
>>> +   /* Check for Arbitration lost interrupt */
>>> +   if (isr & XCAN_IXR_ARBLST_MASK) {
>>> +           cf->can_id |= CAN_ERR_LOSTARB;
>>> +           cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
>>> +           priv->can.can_stats.arbitration_lost++;
>>> +   }
>>> +
>>> +   /* Check for RX FIFO Overflow interrupt */
>>> +   if (isr & XCAN_IXR_RXOFLW_MASK) {
>>> +           cf->can_id |= CAN_ERR_CRTL;
>>> +           cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
>>> +           stats->rx_over_errors++;
>>> +           stats->rx_errors++;
>>> +           priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_RESET_MASK);
>>> +   }
>>> +
>>> +   /* Check for error interrupt */
>>> +   if (isr & XCAN_IXR_ERROR_MASK) {
>>> +           cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +           cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>>> +
>>> +           /* Check for Ack error interrupt */
>>> +           if (err_status & XCAN_ESR_ACKER_MASK) {
>>> +                   cf->can_id |= CAN_ERR_ACK;
>>> +                   cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
>>> +                   stats->tx_errors++;
>>> +           }
>>> +
>>> +           /* Check for Bit error interrupt */
>>> +           if (err_status & XCAN_ESR_BERR_MASK) {
>>> +                   cf->can_id |= CAN_ERR_PROT;
>>> +                   cf->data[2] = CAN_ERR_PROT_BIT;
>>> +                   stats->tx_errors++;
>>> +           }
>>> +
>>> +           /* Check for Stuff error interrupt */
>>> +           if (err_status & XCAN_ESR_STER_MASK) {
>>> +                   cf->can_id |= CAN_ERR_PROT;
>>> +                   cf->data[2] = CAN_ERR_PROT_STUFF;
>>> +                   stats->rx_errors++;
>>> +           }
>>> +
>>> +           /* Check for Form error interrupt */
>>> +           if (err_status & XCAN_ESR_FMER_MASK) {
>>> +                   cf->can_id |= CAN_ERR_PROT;
>>> +                   cf->data[2] = CAN_ERR_PROT_FORM;
>>> +                   stats->rx_errors++;
>>> +           }
>>> +
>>> +           /* Check for CRC error interrupt */
>>> +           if (err_status & XCAN_ESR_CRCER_MASK) {
>>> +                   cf->can_id |= CAN_ERR_PROT;
>>> +                   cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
>>> +                                   CAN_ERR_PROT_LOC_CRC_DEL;
>>> +                   stats->rx_errors++;
>>> +           }
>>> +                   priv->can.can_stats.bus_error++;
>>> +   }
>>> +
>>> +   netif_rx(skb);
>>> +   stats->rx_packets++;
>>> +   stats->rx_bytes += cf->can_dlc;
>>> +
>>> +   netdev_dbg(ndev, "%s: error status register:0x%x\n",
>>> +                   __func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
>> }
>>> +
>>> +/**
>>> + * xcan_state_interrupt - It will check the state of the CAN device
>>> + * @ndev:  net_device pointer
>>> + * @isr:   interrupt status register value
>>> + *
>>> + * This will checks the state of the CAN device
>>> + * and puts the device into appropriate state.
>>> + */
>>> +static void xcan_state_interrupt(struct net_device *ndev, u32 isr) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> +   /* Check for Sleep interrupt if set put CAN device in sleep state */
>>> +   if (isr & XCAN_IXR_SLP_MASK)
>>> +           priv->can.state = CAN_STATE_SLEEPING;
>>> +
>>> +   /* Check for Wake up interrupt if set put CAN device in Active state
>> */
>>> +   if (isr & XCAN_IXR_WKUP_MASK)
>>> +           priv->can.state = CAN_STATE_ERROR_ACTIVE; }
>>> +
>>> +/**
>>> + * xcan_rx_poll - Poll routine for rx packets (NAPI)
>>> + * @napi:  napi structure pointer
>>> + * @quota: Max number of rx packets to be processed.
>>> + *
>>> + * This is the poll routine for rx part.
>>> + * It will process the packets maximux quota value.
>>> + *
>>> + * Return: number of packets received  */ static int
>>> +xcan_rx_poll(struct napi_struct *napi, int quota) {
>>> +   struct net_device *ndev = napi->dev;
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   u32 isr, ier;
>>> +   int work_done = 0;
>>> +
>>> +   isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> +   while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
>>> +           if (isr & XCAN_IXR_RXOK_MASK) {
>>> +                   priv->write_reg(priv, XCAN_ICR_OFFSET,
>>> +                           XCAN_IXR_RXOK_MASK);
>>> +                   if (xcan_rx(ndev) < 0)
>>> +                           return work_done;
>>> +                   work_done++;
>>> +           } else {
>>> +                   priv->write_reg(priv, XCAN_ICR_OFFSET,
>>> +                           XCAN_IXR_RXNEMP_MASK);
>>> +                   break;
>>> +           }
>>> +           priv->write_reg(priv, XCAN_ICR_OFFSET,
>> XCAN_IXR_RXNEMP_MASK);
>>> +           isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> +   }
>>> +
>>> +   if (work_done < quota) {
>>> +           napi_complete(napi);
>>> +           ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> +           ier |= (XCAN_IXR_RXOK_MASK |
>> XCAN_IXR_RXNEMP_MASK);
>>> +           priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> +   }
>>> +   return work_done;
>>> +}
>>> +
>>> +/**
>>> + * xcan_tx_interrupt - Tx Done Isr
>>> + * @ndev:  net_device pointer
>>> + */
>>> +static void xcan_tx_interrupt(struct net_device *ndev) {
>>> +   unsigned long flags;
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   struct net_device_stats *stats = &ndev->stats;
>>> +   u32 processed = 0, txpackets;
>>> +
>>> +   stats->tx_packets++;
>>> +   netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
>>> +                   priv->waiting_ech_skb_num, priv-
>>> waiting_ech_skb_index);
>>> +
>>> +   txpackets = priv->waiting_ech_skb_num;
>>> +
>>> +   if (txpackets) {
>>> +           can_get_echo_skb(ndev, priv->waiting_ech_skb_index);
>>> +           priv->waiting_ech_skb_index =
>>> +                   (priv->waiting_ech_skb_index + 1) %
>>> +                   priv->xcan_echo_skb_max_tx;
>>> +           processed++;
>>> +           txpackets--;
>>> +   }
>>> +
>>> +   spin_lock_irqsave(&priv->ech_skb_lock, flags);
>>> +   priv->waiting_ech_skb_num -= processed;
>>> +   spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>>> +
>>> +   netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
>>> +                   priv->waiting_ech_skb_num, priv-
>>> waiting_ech_skb_index);
>>> +
>>> +   netif_wake_queue(ndev);
>>> +
>>> +   can_led_event(ndev, CAN_LED_EVENT_TX); }
>>> +
>>> +/**
>>> + * xcan_interrupt - CAN Isr
>>> + * @irq:   irq number
>>> + * @dev_id:        device id poniter
>>> + *
>>> + * This is the xilinx CAN Isr. It checks for the type of interrupt
>>> + * and invokes the corresponding ISR.
>>> + *
>>> + * Return:
>>> + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise
>>> +*/ static irqreturn_t xcan_interrupt(int irq, void *dev_id) {
>>> +   struct net_device *ndev = (struct net_device *)dev_id;
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   u32 isr, ier;
>>> +
>>> +   if (priv->can.state == CAN_STATE_STOPPED)
>>
>> This should not happen, please remove.
> 
> Ok
>>
>>> +           return IRQ_NONE;
>>> +
>>> +   /* Get the interrupt status from Xilinx CAN */
>>> +   isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> +   if (!isr)
>>> +           return IRQ_NONE;
>>> +
>>> +   netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__,
>>> +                   isr, priv->read_reg(priv, XCAN_ESR_OFFSET));
>>> +
>>> +   /* Check for the type of interrupt and Processing it */
>>> +   if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) {
>>> +           priv->write_reg(priv, XCAN_ICR_OFFSET,
>> (XCAN_IXR_SLP_MASK |
>>> +                           XCAN_IXR_WKUP_MASK));
>>> +           xcan_state_interrupt(ndev, isr);
>>> +   }
>>> +
>>> +   /* Check for Tx interrupt and Processing it */
>>> +   if (isr & XCAN_IXR_TXOK_MASK) {
>>> +           priv->write_reg(priv, XCAN_ICR_OFFSET,
>> XCAN_IXR_TXOK_MASK);
>>> +           xcan_tx_interrupt(ndev);
>>> +   }
>>> +
>>> +   /* Check for the type of error interrupt and Processing it */
>>> +   if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
>>> +                   XCAN_IXR_BSOFF_MASK |
>> XCAN_IXR_ARBLST_MASK)) {
>>> +           priv->write_reg(priv, XCAN_ICR_OFFSET,
>> (XCAN_IXR_ERROR_MASK |
>>> +                           XCAN_IXR_RXOFLW_MASK |
>> XCAN_IXR_BSOFF_MASK |
>>> +                           XCAN_IXR_ARBLST_MASK));
>>> +           xcan_err_interrupt(ndev, isr);
>>> +   }
>>> +
>>> +   /* Check for the type of receive interrupt and Processing it */
>>> +   if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
>>> +           ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> +           ier &= ~(XCAN_IXR_RXNEMP_MASK |
>> XCAN_IXR_RXOK_MASK);
>>> +           priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> +           napi_schedule(&priv->napi);
>>> +   }
>>> +   return IRQ_HANDLED;
>>> +}
>>> +
>>> +/**
>>> + * xcan_stop - Driver stop routine
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This is the drivers stop routine. It will disable the
>>> + * interrupts and put the device into configuration mode.
>>> + */
>>> +static void xcan_stop(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   u32 ier;
>>> +
>>> +   /* Disable interrupts and leave the can in configuration mode */
>>> +   ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> +   ier &= ~XCAN_INTR_ALL;
>>> +   priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> +   priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
>>> +   priv->can.state = CAN_STATE_STOPPED; }
>>> +
>>> +/**
>>> + * xcan_open - Driver open routine
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * This is the driver open routine.
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_open(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   int err;
>>> +
>>> +   /* Set chip into reset mode */
>>> +   err = set_reset_mode(ndev);
>>> +   if (err < 0)
>>> +           netdev_err(ndev, "mode resetting failed failed!\n");
>>> +
>>> +   /* Common open */
>>> +   err = open_candev(ndev);
>>> +   if (err)
>>> +           return err;
>>> +
>>> +   err = xcan_start(ndev);
>>> +   if (err < 0)
>>> +           netdev_err(ndev, "xcan_start failed!\n");
>>> +
>>> +
>>> +   can_led_event(ndev, CAN_LED_EVENT_OPEN);
>>> +   napi_enable(&priv->napi);
>>> +   netif_start_queue(ndev);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_close - Driver close routine
>>> + * @ndev:  Pointer to net_device structure
>>> + *
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_close(struct net_device *ndev) {
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> +   netif_stop_queue(ndev);
>>> +   napi_disable(&priv->napi);
>>> +   xcan_stop(ndev);
>>> +   close_candev(ndev);
>>> +
>>> +   can_led_event(ndev, CAN_LED_EVENT_STOP);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_get_berr_counter - error counter routine
>>> + * @ndev:  Pointer to net_device structure
>>> + * @bec:   Pointer to can_berr_counter structure
>>> + *
>>> + * This is the driver error counter routine.
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_get_berr_counter(const struct net_device *ndev,
>>> +                                   struct can_berr_counter *bec)
>>> +{
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> +   bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) &
>> XCAN_ECR_TEC_MASK;
>>> +   bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
>>> +                   XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
>>> +   return 0;
>>> +}
>>> +
>>> +static const struct net_device_ops xcan_netdev_ops = {
>>> +   .ndo_open       = xcan_open,
>>> +   .ndo_stop       = xcan_close,
>>> +   .ndo_start_xmit = xcan_start_xmit,
>>> +};
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +/**
>>> + * xcan_suspend - Suspend method for the driver
>>> + * @_dev:  Address of the platform_device structure
>>> + *
>>> + * Put the driver into low power mode.
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_suspend(struct device *_dev) {
>>> +   struct platform_device *pdev = container_of(_dev,
>>> +                   struct platform_device, dev);
>>> +   struct net_device *ndev = platform_get_drvdata(pdev);
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> +   if (netif_running(ndev)) {
>>> +           netif_stop_queue(ndev);
>>> +           netif_device_detach(ndev);
>>> +   }
>>> +
>>> +   priv->write_reg(priv, XCAN_MSR_OFFSET,
>> XCAN_MSR_SLEEP_MASK);
>>> +   priv->can.state = CAN_STATE_SLEEPING;
>>> +
>>> +   clk_disable(priv->aperclk);
>>> +   clk_disable(priv->devclk);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_resume - Resume from suspend
>>> + * @dev:   Address of the platformdevice structure
>>> + *
>>> + * Resume operation after suspend.
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_resume(struct device *dev) {
>>> +   struct platform_device *pdev = container_of(dev,
>>> +                   struct platform_device, dev);
>>> +   struct net_device *ndev = platform_get_drvdata(pdev);
>>> +   struct xcan_priv *priv = netdev_priv(ndev);
>>> +   int ret;
>>> +
>>> +   ret = clk_enable(priv->aperclk);
>>> +   if (ret) {
>>> +           dev_err(dev, "Cannot enable clock.\n");
>>> +           return ret;
>>> +   }
>>> +   ret = clk_enable(priv->devclk);
>>> +   if (ret) {
>>> +           dev_err(dev, "Cannot enable clock.\n");
>>> +           return ret;
>>> +   }
>>> +
>>> +   priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> +   priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>> +   priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +
>>> +   if (netif_running(ndev)) {
>>> +           netif_device_attach(ndev);
>>> +           netif_start_queue(ndev);
>>> +   }
>>> +
>>> +   return 0;
>>> +}
>>> +#endif
>>> +
>>> +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
>> xcan_resume);
>>> +
>>> +/**
>>> + * xcan_probe - Platform registration call
>>> + * @pdev:  Handle to the platform device structure
>>> + *
>>> + * This function does all the memory allocation and registration for
>>> +the CAN
>>> + * device.
>>> + *
>>> + * Return: 0 on success and failure value on error  */ static int
>>> +xcan_probe(struct platform_device *pdev) {
>>> +   struct resource *res; /* IO mem resources */
>>> +   struct net_device *ndev;
>>> +   struct xcan_priv *priv;
>>> +   int ret, fifodep;
>>> +
>>> +   /* Create a CAN device instance */
>>> +   ndev = alloc_candev(sizeof(struct xcan_priv),
>> XCAN_ECHO_SKB_MAX);
>>> +   if (!ndev)
>>> +           return -ENOMEM;
>>> +
>>> +   priv = netdev_priv(ndev);
>>> +   priv->dev = ndev;
>>> +   priv->can.bittiming_const = &xcan_bittiming_const;
>>> +   priv->can.do_set_bittiming = xcan_set_bittiming;
>>> +   priv->can.do_set_mode = xcan_do_set_mode;
>>> +   priv->can.do_get_berr_counter = xcan_get_berr_counter;
>>> +   priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>>> +                                   CAN_CTRLMODE_BERR_REPORTING;
>>> +   priv->xcan_echo_skb_max_tx = XCAN_ECHO_SKB_MAX;
>>> +   priv->xcan_echo_skb_max_rx = XCAN_NAPI_WEIGHT;
>>> +
>>> +   /* Get IRQ for the device */
>>> +   ndev->irq = platform_get_irq(pdev, 0);
>>> +   ret = devm_request_irq(&pdev->dev, ndev->irq, &xcan_interrupt,
>>> +                           priv->irq_flags, dev_name(&pdev->dev),
>>> +                           (void *)ndev);
>>
>> We usually request the interrupt on in the open() function
>>
> 
> 
> Ok Will Move into open routine

This means you cannot use devm_ here and you have to tear down the
interrupt handler in the close function.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140207/bf647d69/attachment-0001.sig>

^ permalink raw reply

* [PATCH v3 04/15] ARM: at91: prepare common clk transition for sam9261 SoC
From: Jean-Jacques Hiblot @ 2014-02-07  9:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52F49AEB.6060307@overkiz.com>

Boris is right, I shamelessly re-used his work with no thought for
modification. BTW thank you Boris for the neat job


2014-02-07 Boris BREZILLON <b.brezillon@overkiz.com>:
> Hello Jean-Christophe,
>
>
> On 07/02/2014 09:25, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>> On 16:57 Thu 23 Jan     , Jean-Jacques Hiblot wrote:
>>>
>>>   This patch encloses sam9261 old clk registration in
>>> "#if defined(CONFIG_OLD_CLK_AT91) #endif" sections.
>>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> ---
>>>   arch/arm/mach-at91/at91sam9261.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/at91sam9261.c
>>> b/arch/arm/mach-at91/at91sam9261.c
>>> index 6a2c869..2c0e940 100644
>>> --- a/arch/arm/mach-at91/at91sam9261.c
>>> +++ b/arch/arm/mach-at91/at91sam9261.c
>>> @@ -25,10 +25,12 @@
>>>   #include "at91_rstc.h"
>>>   #include "soc.h"
>>>   #include "generic.h"
>>> -#include "clock.h"
>>>   #include "sam9_smc.h"
>>>   #include "pm.h"
>>>   +#if defined(CONFIG_OLD_CLK_AT91)
>>> +#include "clock.h"
>>> +
>>>   /* --------------------------------------------------------------------
>>>    *  Clocks
>>>    * --------------------------------------------------------------------
>>> */
>>> @@ -262,7 +264,7 @@ static void __init at91sam9261_register_clocks(void)
>>>         clk_register(&hck0);
>>>         clk_register(&hck1);
>>>   }
>>> -
>>
>> do this here
>>
>> #else
>> #define at91sam9261_register_clocks NULL
>>>
>>> +#endif
>>>   /* --------------------------------------------------------------------
>>>    *  GPIO
>>>    * --------------------------------------------------------------------
>>> */
>>> @@ -362,6 +364,8 @@ AT91_SOC_START(at91sam9261)
>>>         .extern_irq = (1 << AT91SAM9261_ID_IRQ0) | (1 <<
>>> AT91SAM9261_ID_IRQ1)
>>>                     | (1 << AT91SAM9261_ID_IRQ2),
>>>         .ioremap_registers = at91sam9261_ioremap_registers,
>>> +#if defined(CONFIG_OLD_CLK_AT91)
>>>         .register_clocks = at91sam9261_register_clocks,
>>> +#endif
>>
>> so no ifdef here
>
>
> I guess he did this based on what I did for the sama5 SoC, but you're right:
> your proposal is cleaner.
>
> Best Regards,
>
> Boris
>
>
>>>         .init = at91sam9261_initialize,
>>>   AT91_SOC_END
>>> --
>>> 1.8.5.2
>>>
>

^ permalink raw reply

* [PATCH] backlight: add PWM dependencies
From: Arnd Bergmann @ 2014-02-07  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <000d01cf23b1$7c1f8de0$745ea9a0$%han@samsung.com>

On Friday 07 February 2014, Jingoo Han wrote:
> How about the following?
> 
>   [PATCH 1/7] ARM: pxa: don't select HAVE_PWM
>   [PATCH 2/7] ARM: lpc32xx: don't select HAVE_PWM
>   [PATCH 3/7] ARM: remove HAVE_PWM config option
>   [PATCH 4/7] MIPS: jz4740: don't select HAVE_PWM
>   [PATCH 5/7] Input: max8997_haptic: remove HAVE_PWM dependencies
>   [PATCH 6/7] Input: pwm-beepe: remove HAVE_PWM dependencies
>   [PATCH 7/7] pwm: don't use IS_ENABLED(CONFIG_HAVE_PWM)
> 
> I would like to merge it through PWM tree.
> After merging these patches, all HAVE_PWM will be removed from
> the mainline kernel. Thank you. :-)

Sounds godo to me, thanks a lot for taking care of this!
I don't see any inter-dependencies between the various patches,
so we could also take the first three through the arm-soc tree
to avoid conflicts with other changes (or possibly the third
one through rmk's ARM tree, if he prefers).

	Arnd

^ permalink raw reply

* [PATCH 19/21] ARM: MVEBU: Simplifiy headers and make local
From: Andrew Lunn @ 2014-02-07  9:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140207103631.44789e1d@skate>

On Fri, Feb 07, 2014 at 10:36:31AM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Fri, 7 Feb 2014 10:20:47 +0100, Andrew Lunn wrote:
> 
> > I wanted to keep 14/21 as a simple move of files, doing the minimal
> > needed to make it boot. The patches that follow then clean up. I found
> > that simpler to follow what was going on, since there is nothing
> > hidden in the move patch.
> > 
> > I can however squash these together if that is what people want to
> > see.
> 
> Ok. Then maybe the commit log should indicate this (that the commit is
> only a move and that followup commits clean things up).

Hi Thomas

The commit log for 14/21 currently is:

Move the kirkwood DT support into mach-mvebu, and make them part of                                              
ARCH_MULTI_V5. Minimal changes have been made in order to make it                                                
boot. More integration will take place in following patches. 

I could change "integration" to "integration and cleanup".

  Andrew

^ permalink raw reply

* [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Deepthi Dharwar @ 2014-02-07  9:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52F484D9.6020604@linux.vnet.ibm.com>

Hi Preeti,

Thanks for the patch.

On 02/07/2014 12:31 PM, Preeti U Murthy wrote:
> Hi Nicolas,
> 
> Find below the patch that will need to be squashed with this one.
> This patch is based on the mainline.Adding Deepthi, the author of
> the patch which introduced the powernv cpuidle driver. Deepthi,
> do you think the below patch looks right? We do not need to do an
> explicit local_irq_enable() since we are in the call path of
> cpuidle driver and that explicitly enables irqs on exit from
> idle states.

Yes, We enable irqs explicitly while entering snooze loop and we always
have interrupts enabled in the snooze state.
For NAP state, we exit out of this state with interrupts enabled so we
do not need an explicit enable of irqs.

> On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
>> On Thu, 6 Feb 2014, Preeti U Murthy wrote:
>>
>>> Hi Daniel,
>>>
>>> On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
>>>> Hi Nico,
>>>>
>>>>
>>>> On 6 February 2014 14:16, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>>>>
>>>>> The core idle loop now takes care of it.
>>>>>
>>>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>>>> ---
>>>>>  arch/powerpc/platforms/powernv/setup.c | 13 +------------
>>>>>  1 file changed, 1 insertion(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/platforms/powernv/setup.c
>>>>> b/arch/powerpc/platforms/powernv/setup.c
>>>>> index 21166f65c9..a932feb290 100644
>>>>> --- a/arch/powerpc/platforms/powernv/setup.c
>>>>> +++ b/arch/powerpc/platforms/powernv/setup.c
>>>>> @@ -26,7 +26,6 @@
>>>>>  #include <linux/of_fdt.h>
>>>>>  #include <linux/interrupt.h>
>>>>>  #include <linux/bug.h>
>>>>> -#include <linux/cpuidle.h>
>>>>>
>>>>>  #include <asm/machdep.h>
>>>>>  #include <asm/firmware.h>
>>>>> @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
>>>>>         return 1;
>>>>>  }
>>>>>
>>>>> -void powernv_idle(void)
>>>>> -{
>>>>> -       /* Hook to cpuidle framework if available, else
>>>>> -        * call on default platform idle code
>>>>> -        */
>>>>> -       if (cpuidle_idle_call()) {
>>>>> -               power7_idle();
>>>>> -       }
>>>>>
> 
>  drivers/cpuidle/cpuidle-powernv.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 78fd174..130f081 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -31,11 +31,13 @@ static int snooze_loop(struct cpuidle_device *dev,
>  	set_thread_flag(TIF_POLLING_NRFLAG);
> 
>  	while (!need_resched()) {
> +		ppc64_runlatch_off();
                ^^^^^^^^^^^^^^^
We could move this before the while() loop.
It would ideal to turn off latch when we enter snooze and
turn it on when we are about to exit, rather than doing
it over and over in the while loop.

>  		HMT_low();
>  		HMT_very_low();
>  	}
> 
>  	HMT_medium();
> +	ppc64_runlatch_on();
>  	clear_thread_flag(TIF_POLLING_NRFLAG);
>  	smp_mb();
>  	return index;
> @@ -45,7 +47,9 @@ static int nap_loop(struct cpuidle_device *dev,
>  			struct cpuidle_driver *drv,
>  			int index)
>  {
> +	ppc64_runlatch_off();
>  	power7_idle();
> +	ppc64_runlatch_on();
>  	return index;
>  }
> 
> Thanks
> 
> Regards
> Preeti U Murthy
> 

Regards,
Deepthi

^ permalink raw reply

* [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
From: Russell King - ARM Linux @ 2014-02-07  9:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKMK7uFYhz8Pmv5E7aKY7yzZGDe_m8a0382Njv7tZRoBSfmRpw@mail.gmail.com>

On Fri, Feb 07, 2014 at 10:04:30AM +0100, Daniel Vetter wrote:
> I've chatted a bit with Hans Verkuil about this topic at fosdem and
> apparently both v4l and alsa have something like this already in their
> helper libraries. Adding more people as fyi in case they want to
> switch to the new driver core stuff from Russell.

It's not ALSA, but ASoC which has this.  Mark is already aware of this
and will be looking at it from an ASoC perspective.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v3 01/15] at91: dt: Add at91sam9261 dt SoC support
From: Jean-Jacques Hiblot @ 2014-02-07  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52F49CBE.4030508@overkiz.com>

2014-02-07 Boris BREZILLON <b.brezillon@overkiz.com>:
> Hello Jean-Christophe,
>
>
> On 07/02/2014 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>> On 16:57 Thu 23 Jan     , Jean-Jacques Hiblot wrote:
>>>
>>> This patch adds the basics to support the Device Tree on a sam9261-based
>>> platform
>>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> ---
>>>   arch/arm/boot/dts/at91sam9261.dtsi | 627
>>> +++++++++++++++++++++++++++++++++++++
>>>   arch/arm/mach-at91/at91sam9261.c   |  15 +
>>>   2 files changed, 642 insertions(+)
>>>   create mode 100644 arch/arm/boot/dts/at91sam9261.dtsi
>>>
>>> diff --git a/arch/arm/boot/dts/at91sam9261.dtsi
>>> b/arch/arm/boot/dts/at91sam9261.dtsi
>>> new file mode 100644
>>> index 0000000..2730611
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/at91sam9261.dtsi
>>> @@ -0,0 +1,627 @@
>>> +/*
>>> + * at91sam9261.dtsi - Device Tree Include file for AT91SAM9261 SoC
>>> + *
>>> + *  Copyright (C) 2013 Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> + *
>>> + * Licensed under GPLv2 only.
>>> + */
>>> +
>>> +#include "skeleton.dtsi"
>>> +#include <dt-bindings/pinctrl/at91.h>
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/clk/at91.h>
>>> +
>>> +/ {
>>> +       model = "Atmel AT91SAM9261 family SoC";
>>> +       compatible = "atmel,at91sam9261";
>>> +       interrupt-parent = <&aic>;
>>> +
>>> +       aliases {
>>> +               serial0 = &dbgu;
>>> +               serial1 = &usart0;
>>> +               serial2 = &usart1;
>>> +               serial3 = &usart2;
>>> +               gpio0 = &pioA;
>>> +               gpio1 = &pioB;
>>> +               gpio2 = &pioC;
>>> +               tcb0 = &tcb0;
>>> +               i2c0 = &i2c0;
>>> +               ssc0 = &ssc0;
>>> +               ssc1 = &ssc1;
>>> +       };
>>> +       cpus {
>>> +               #address-cells = <0>;
>>> +               #size-cells = <0>;
>>> +
>>> +               cpu {
>>> +                       compatible = "arm,arm926ej-s";
>>> +                       device_type = "cpu";
>>> +               };
>>> +       };
>>> +
>>> +       memory {
>>> +               reg = <0x20000000 0x08000000>;
>>> +       };
>>> +
>>> +       ahb {
>>> +               compatible = "simple-bus";
>>> +               #address-cells = <1>;
>>> +               #size-cells = <1>;
>>> +               ranges;
>>> +
>>> +               usb0: ohci at 00500000 {
>>> +                       compatible = "atmel,at91rm9200-ohci", "usb-ohci";
>>> +                       reg = <0x00500000 0x100000>;
>>> +                       interrupts = <20 IRQ_TYPE_LEVEL_HIGH 2>;
>>
>> as requested before use the new interrupt property
>> interrupts-extended (mandatory)
>>
>> there is missing
>>                         pinctrl-names = "default";
>>
>> on mmc & co
>>>
>>> +                       status = "disabled";
>>> +               };
>>> +
>>> +               nand0: nand at 40000000 {
>>> +                       compatible = "atmel,at91rm9200-nand";
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <1>;
>>> +                       reg = <0x40000000 0x10000000>;
>>> +                       atmel,nand-addr-offset = <22>;
>>> +                       atmel,nand-cmd-offset = <21>;
>>> +                       pinctrl-names = "default";
>>> +                       pinctrl-0 = <&pinctrl_nand>;
>>> +
>>> +                       gpios = <&pioC 15 GPIO_ACTIVE_HIGH
>>> +                               &pioC 14 GPIO_ACTIVE_HIGH
>>> +                               0
>>> +                               >;
>>> +                       status = "disabled";
>>> +               };
>>> +
>>> +               apb {
>>> +                       compatible = "simple-bus";
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <1>;
>>> +                       ranges;
>>> +
>>> +                       tcb0: timer at fffa0000 {
>>> +                               compatible = "atmel,at91rm9200-tcb";
>>> +                               reg = <0xfffa0000 0x100>;
>>> +                               interrupts = < 17 IRQ_TYPE_LEVEL_HIGH 0
>>> +                                       18 IRQ_TYPE_LEVEL_HIGH 0
>>> +                                       19 IRQ_TYPE_LEVEL_HIGH 0
>>> +                                       >;
>>> +                               clocks = <&tcb0_clk>;
>>> +                               clock-names = "t0_clk";
>>> +                               status = "disabled";
>>
>> we use the tcb as shedule clock this should be on by default
>>>
>>> +                       };
>>> +
>>> +                       usb1: gadget at fffa4000 {
>>> +                               compatible = "atmel,at91rm9200-udc";
>>> +                               reg = <0xfffa4000 0x4000>;
>>> +                               interrupts = <10 IRQ_TYPE_LEVEL_HIGH 2>;
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       mmc0: mmc at fffa8000 {
>>> +                               compatible = "atmel,hsmci";
>>> +                               reg = <0xfffa8000 0x600>;
>>> +                               interrupts = <9 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               clocks = <&mci0_clk>;
>>> +                               clock-names = "mci_clk";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       i2c0: i2c at fffac000 {
>>> +                               compatible = "atmel,at91sam9261-i2c";
>>> +                               pinctrl-0 = <&pinctrl_i2c_twi>;
>>> +                               reg = <0xfffac000 0x100>;
>>> +                               interrupts = <11 IRQ_TYPE_LEVEL_HIGH 6>;
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               clocks = <&twi0_clk>;
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       usart0: serial at fffb0000 {
>>> +                               compatible = "atmel,at91sam9260-usart";
>>> +                               reg = <0xfffb0000 0x200>;
>>> +                               interrupts = <6 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +                               atmel,use-dma-rx;
>>> +                               atmel,use-dma-tx;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_usart0>;
>>> +                               clocks = <&usart0_clk>;
>>> +                               clock-names = "usart";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       usart1: serial at fffb4000 {
>>> +                               compatible = "atmel,at91sam9260-usart";
>>> +                               reg = <0xfffb4000 0x200>;
>>> +                               interrupts = <7 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +                               atmel,use-dma-rx;
>>> +                               atmel,use-dma-tx;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_usart1>;
>>> +                               clocks = <&usart1_clk>;
>>> +                               clock-names = "usart";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       usart2: serial at fffb8000{
>>> +                               compatible = "atmel,at91sam9260-usart";
>>> +                               reg = <0xfffb8000 0x200>;
>>> +                               interrupts = <8 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +                               atmel,use-dma-rx;
>>> +                               atmel,use-dma-tx;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_usart2>;
>>> +                               clocks = <&usart2_clk>;
>>> +                               clock-names = "usart";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       ssc0: ssc at fffbc000 {
>>> +                               compatible = "atmel,at91rm9200-ssc";
>>> +                               reg = <0xfffbc000 0x4000>;
>>> +                               interrupts = <14 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_ssc0_tx
>>> &pinctrl_ssc0_rx>;
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       ssc1: ssc at fffc0000 {
>>> +                               compatible = "atmel,at91rm9200-ssc";
>>> +                               reg = <0xfffc0000 0x4000>;
>>> +                               interrupts = <15 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_ssc1_tx
>>> &pinctrl_ssc1_rx>;
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       spi0: spi at fffc8000 {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               compatible = "atmel,at91rm9200-spi";
>>> +                               reg = <0xfffc8000 0x200>;
>>> +                               interrupts = <12 IRQ_TYPE_LEVEL_HIGH 3>;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_spi0>;
>>> +                               clocks = <&spi0_clk>;
>>> +                               clock-names = "spi_clk";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       spi1: spi at fffcc000 {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               compatible = "atmel,at91rm9200-spi";
>>> +                               reg = <0xfffcc000 0x200>;
>>> +                               interrupts = <13 IRQ_TYPE_LEVEL_HIGH 3>;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_spi1>;
>>> +                               clocks = <&spi1_clk>;
>>> +                               clock-names = "spi_clk";
>>> +                               status = "disabled";
>>> +                       };
>>
>> as said for sama5d3 we need to have macro for the hw CS as GPIO to make it
>> more clear to use and read
>>>
>>> +
>>> +                       ramc: ramc at ffffea00 {
>>> +                               compatible = "atmel,at91sam9260-sdramc";
>>> +                               reg = <0xffffea00 0x200>;
>>> +                       };
>>> +
>>> +                       aic: interrupt-controller at fffff000 {
>>> +                               #interrupt-cells = <3>;
>>> +                               compatible = "atmel,at91rm9200-aic";
>>> +                               interrupt-controller;
>>> +                               reg = <0xfffff000 0x200>;
>>> +                               atmel,external-irqs = <29 30 31>;
>>> +                       };
>>
>> one missing empty line
>>>
>>> +                       dbgu: serial at fffff200 {
>>> +                               compatible = "atmel,at91sam9260-usart";
>>> +                               reg = <0xfffff200 0x200>;
>>> +                               interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
>>> +                               pinctrl-names = "default";
>>> +                               pinctrl-0 = <&pinctrl_dbgu>;
>>> +                               clocks = <&mck>;
>>> +                               clock-names = "usart";
>>> +                               status = "disabled";
>>> +                       };
>>> +
>>> +                       pinctrl at fffff400 {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <1>;
>>> +                               compatible = "atmel,at91rm9200-pinctrl",
>>> "simple-bus";
>>> +                               ranges = <0xfffff400 0xfffff400 0xa00>;
>>> +
>>> +                               atmel,mux-mask = <
>>> +                                     /*    A         B     */
>>> +                                      0xffffffff 0xfffffff7  /* pioA */
>>> +                                      0xffffffff 0xfffffff4  /* pioB */
>>> +                                      0xffffffff 0xffffff07  /* pioC */
>>> +                                     >;
>>> +
>>> +                               /* shared pinctrl settings */
>>> +                               dbgu {
>>> +                                       pinctrl_dbgu: dbgu-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 9
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 10
>>> AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               usart0 {
>>> +                                       pinctrl_usart0: usart0-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 8
>>> AT91_PERIPH_A AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOC 9
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart0_rts: usart0_rts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 10
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart0_cts: usart0_cts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 11
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               usart1 {
>>> +                                       pinctrl_usart1: usart1-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 12
>>> AT91_PERIPH_A AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOC 13
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart1_rts: usart1_rts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 12
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart1_cts: usart1_cts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 13
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               usart2 {
>>> +                                       pinctrl_usart2: usart2-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 14
>>> AT91_PERIPH_A AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOC 15
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart2_rts: usart2_rts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 15
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_usart2_cts: usart2_cts-0
>>> {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 16
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               nand {
>>> +                                       pinctrl_nand: nand-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOC 15
>>> AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOC 14
>>> AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               mmc0 {
>>> +                                       pinctrl_mmc0_clk: mmc0_clk-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 2
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_mmc0_slot0_cmd_dat0:
>>> mmc0_slot0_cmd_dat0-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 1
>>> AT91_PERIPH_B AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOA 0
>>> AT91_PERIPH_B AT91_PINCTRL_PULL_UP>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_mmc0_slot0_dat1_3:
>>> mmc0_slot0_dat1_3-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 4
>>> AT91_PERIPH_B AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOA 5
>>> AT91_PERIPH_B AT91_PINCTRL_PULL_UP
>>> +                                                        AT91_PIOA 6
>>> AT91_PERIPH_B AT91_PINCTRL_PULL_UP>;
>>> +                                       };
>>> +                                       };
>>> +
>>> +                               ssc0 {
>>> +                                       pinctrl_ssc0_tx: ssc0_tx-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOB 21
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 22
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 23
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_ssc0_rx: ssc0_rx-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOB 24
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 25
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 26
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               ssc1 {
>>> +                                       pinctrl_ssc1_tx: ssc1_tx-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 17
>>> AT91_PERIPH_B AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 18
>>> AT91_PERIPH_B AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 19
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_ssc1_rx: ssc1_rx-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 20
>>> AT91_PERIPH_B AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 21
>>> AT91_PERIPH_B AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 22
>>> AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               spi0 {
>>> +                                       pinctrl_spi0: spi0-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 0
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 1
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOA 2
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                                       };
>>> +
>>> +                               spi1 {
>>> +                                       pinctrl_spi1: spi1-0 {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOB 30
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 31
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                        AT91_PIOB 29
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               tcb0 {
>>> +                                       pinctrl_tcb0_tclk0: tcb0_tclk0-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 16 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tclk1: tcb0_tclk1-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 17 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tclk2: tcb0_tclk2-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 18 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tioa0: tcb0_tioa0-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 19 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tioa1: tcb0_tioa1-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 21 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tioa2: tcb0_tioa2-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 23 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tiob0: tcb0_tiob0-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 20 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tiob1: tcb0_tiob1-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 22 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +
>>> +                                       pinctrl_tcb0_tiob2: tcb0_tiob2-0
>>> {
>>> +                                               atmel,pins = <AT91_PIOC
>>> 24 AT91_PERIPH_B AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               i2c0 {
>>> +                                       pinctrl_i2c_bitbang:
>>> i2c-0-bitbang {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 7
>>> AT91_PERIPH_GPIO AT91_PINCTRL_NONE
>>> +                                                       AT91_PIOA 8
>>> AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                                       pinctrl_i2c_twi: i2c-0-twi {
>>> +                                               atmel,pins =
>>> +                                                       <AT91_PIOA 7
>>> AT91_PERIPH_A AT91_PINCTRL_NONE
>>> +                                                       AT91_PIOA 8
>>> AT91_PERIPH_A AT91_PINCTRL_NONE>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               pioA: gpio at fffff400 {
>>> +                                       compatible =
>>> "atmel,at91rm9200-gpio";
>>> +                                       reg = <0xfffff400 0x200>;
>>> +                                       interrupts = <2
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                       #gpio-cells = <2>;
>>> +                                       gpio-controller;
>>> +                                       interrupt-controller;
>>> +                                       #interrupt-cells = <2>;
>>> +                                       clocks = <&pioA_clk>;
>>> +                               };
>>> +
>>> +                               pioB: gpio at fffff600 {
>>> +                                       compatible =
>>> "atmel,at91rm9200-gpio";
>>> +                                       reg = <0xfffff600 0x200>;
>>> +                                       interrupts = <3
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                       #gpio-cells = <2>;
>>> +                                       gpio-controller;
>>> +                                       interrupt-controller;
>>> +                                       #interrupt-cells = <2>;
>>> +                                       clocks = <&pioB_clk>;
>>> +                               };
>>> +
>>> +                               pioC: gpio at fffff800 {
>>> +                                       compatible =
>>> "atmel,at91rm9200-gpio";
>>> +                                       reg = <0xfffff800 0x200>;
>>> +                                       interrupts = <4
>>> IRQ_TYPE_LEVEL_HIGH 1>;
>>> +                                       #gpio-cells = <2>;
>>> +                                       gpio-controller;
>>> +                                       interrupt-controller;
>>> +                                       #interrupt-cells = <2>;
>>> +                                       clocks = <&pioC_clk>;
>>> +                               };
>>> +                       };
>>> +
>>> +                       pmc: pmc at fffffc00 {
>>> +                               compatible = "atmel,at91rm9200-pmc";
>>> +                               reg = <0xfffffc00 0x100>;
>>> +                               interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
>>> +                               interrupt-controller;
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               #interrupt-cells = <1>;
>>> +
>>> +                               clk32k: slck {
>>> +                                       compatible = "fixed-clock";
>>> +                                       #clock-cells = <0>;
>>> +                                       clock-frequency = <32768>;
>>> +                               };
>>> +
>>> +                               main: mainck {
>>> +                                       compatible =
>>> "atmel,at91rm9200-clk-main";
>>> +                                       #clock-cells = <0>;
>>> +                                       interrupts-extended = <&pmc
>>> AT91_PMC_MOSCS>;
>>> +                                       clocks = <&clk32k>;
>>> +                               };
>>> +
>>> +                               plla: pllack {
>>> +                                       compatible =
>>> "atmel,at91rm9200-clk-pll";
>>> +                                       #clock-cells = <0>;
>>> +                                       interrupts-extended = <&pmc
>>> AT91_PMC_LOCKA>;
>>> +                                       clocks = <&main>;
>>> +                                       reg = <0>;
>>> +                                       atmel,clk-input-range = <1000000
>>> 32000000>;
>>> +                                       #atmel,pll-clk-output-range-cells
>>> = <4>;
>>> +                                       atmel,pll-clk-output-ranges =
>>> <80000000 200000000 190000000 240000000>;
>>> +                               };
>>> +
>>> +                               pllb: pllbck {
>>> +                                       compatible =
>>> "atmel,at91rm9200-clk-pll";
>>> +                                       #clock-cells = <0>;
>>> +                                       interrupts-extended = <&pmc
>>> AT91_PMC_LOCKB>;
>>> +                                       clocks = <&main>;
>>> +                                       reg = <1>;
>>> +                                       atmel,clk-input-range = <1000000
>>> 32000000>;
>>> +                                       #atmel,pll-clk-output-range-cells
>>> = <4>;
>>> +                                       atmel,pll-clk-output-ranges =
>>> <80000000 200000000 190000000 240000000>;
>>> +                               };
>>> +
>>> +                               mck: masterck {
>>> +                                       compatible =
>>> "atmel,at91rm9200-clk-master";
>>> +                                       #clock-cells = <0>;
>>> +                                       interrupts-extended = <&pmc
>>> AT91_PMC_MCKRDY>;
>>> +                                       clocks = <&clk32k>, <&main>,
>>> <&plla>, <&pllb>;
>>> +                                       atmel,clk-output-range = <0
>>> 94000000>;
>>> +                                       atmel,clk-divisors = <1 2 4 3>;
>>> +                               };
>>> +                               periphck {
>>> +                                       compatible =
>>> "atmel,at91rm9200-clk-peripheral";
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       clocks = <&mck>;
>>> +
>>> +                                       pioA_clk: pioA_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <2>;
>>> +                                       };
>>> +
>>> +                                       pioB_clk: pioB_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <3>;
>>> +                                       };
>>> +
>>> +                                       pioC_clk: pioC_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <4>;
>>> +                                       };
>>> +
>>> +                                       usart0_clk: usart0_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <6>;
>>> +                                       };
>>> +
>>> +                                       usart1_clk: usart1_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <7>;
>>> +                                       };
>>> +
>>> +                                       usart2_clk: usart2_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <8>;
>>> +                                       };
>>> +
>>> +                                       mci0_clk: mci0_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <9>;
>>> +                                       };
>>> +
>>> +                                       twi0_clk: twi0_clk {
>>> +                                               reg = <11>;
>>> +                                               #clock-cells = <0>;
>>> +                                       };
>>> +
>>> +                                       spi0_clk: spi0_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <12>;
>>> +                                       };
>>> +
>>> +                                       spi1_clk: spi1_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <13>;
>>> +                                       };
>>> +
>>> +                                       tcb0_clk: tcb0_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <17>;
>>> +                                       };
>>> +
>>> +                                       lcd_clk: lcd_clk {
>>> +                                               #clock-cells = <0>;
>>> +                                               reg = <21>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +
>>> +                       rstc at fffffd00 {
>>> +                               compatible = "atmel,at91sam9260-rstc";
>>> +                               reg = <0xfffffd00 0x10>;
>>> +                       };
>>> +
>>> +                       shdwc at fffffd10 {
>>> +                               compatible = "atmel,at91sam9260-shdwc";
>>> +                               reg = <0xfffffd10 0x10>;
>>> +                       };
>>> +
>>> +                       pit: timer at fffffd30 {
>>> +                               compatible = "atmel,at91sam9260-pit";
>>> +                               reg = <0xfffffd30 0xf>;
>>> +                               interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
>>> +                               clocks = <&mck>;
>>> +                       };
>>> +
>>> +                       watchdog at fffffd40 {
>>> +                               compatible = "atmel,at91sam9260-wdt";
>>> +                               reg = <0xfffffd40 0x10>;
>>> +                               status = "disabled";
>>> +                       };
>>> +               };
>>> +       };
>>> +
>>> +       i2c at 0 {
>>> +               compatible = "i2c-gpio";
>>> +               pinctrl-0 = <&pinctrl_i2c_bitbang>;
>>> +               gpios = <&pioA 7 GPIO_ACTIVE_HIGH /* sda */
>>> +                        &pioA 8 GPIO_ACTIVE_HIGH /* scl */
>>> +                       >;
>>> +               i2c-gpio,sda-open-drain;
>>> +               i2c-gpio,scl-open-drain;
>>> +               i2c-gpio,delay-us = <2>;        /* ~100 kHz */
>>> +               #address-cells = <1>;
>>> +               #size-cells = <0>;
>>> +               status = "disabled";
>>> +       };
>>> +};
>>> diff --git a/arch/arm/mach-at91/at91sam9261.c
>>> b/arch/arm/mach-at91/at91sam9261.c
>>> index 6276b4c..6a2c869 100644
>>> --- a/arch/arm/mach-at91/at91sam9261.c
>>> +++ b/arch/arm/mach-at91/at91sam9261.c
>>> @@ -189,6 +189,21 @@ static struct clk_lookup periph_clocks_lookups[] = {
>>>         CLKDEV_CON_ID("pioA", &pioA_clk),
>>>         CLKDEV_CON_ID("pioB", &pioB_clk),
>>>         CLKDEV_CON_ID("pioC", &pioC_clk),
>>> +       /* more usart lookup table for DT entries */
>>> +       CLKDEV_CON_DEV_ID("usart", "fffff200.serial", &mck),
>>> +       CLKDEV_CON_DEV_ID("usart", "fffb0000.serial", &usart0_clk),
>>> +       CLKDEV_CON_DEV_ID("usart", "ffffb400.serial", &usart1_clk),
>>> +       CLKDEV_CON_DEV_ID("usart", "fff94000.serial", &usart2_clk),
>>> +       /* more tc lookup table for DT entries */
>>> +       CLKDEV_CON_DEV_ID("t0_clk", "fffa0000.timer", &tc0_clk),
>>> +       CLKDEV_CON_DEV_ID("hclk", "500000.ohci", &hck0),
>>> +       CLKDEV_CON_DEV_ID("spi_clk", "fffc8000.spi", &spi0_clk),
>>> +       CLKDEV_CON_DEV_ID("spi_clk", "fffcc000.spi", &spi1_clk),
>>> +       CLKDEV_CON_DEV_ID("mci_clk", "fffa8000.mmc", &mmc_clk),
>>> +       CLKDEV_CON_DEV_ID(NULL, "fffac000.i2c", &twi_clk),
>>> +       CLKDEV_CON_DEV_ID(NULL, "fffff400.gpio", &pioA_clk),
>>> +       CLKDEV_CON_DEV_ID(NULL, "fffff600.gpio", &pioB_clk),
>>> +       CLKDEV_CON_DEV_ID(NULL, "fffff800.gpio", &pioC_clk),
>>
>> do we really need this with ccf?
>
>
> We need this in case we want to support multi-board kernel with some boards
> that do not support CCF.
>
> If you want to drop these clk lookup defintions you'll have to add the
> "depends on COMMON_CLK" line in the "config SOC_AT91SAM9261" section.
I don't think that non-CCF code can be dropped so fast. Let the
at91sam9261's users be able to use CCF and DT and also the platform
file for a few kernel versions.
Then the platform file and non-CCF support can be dropped after every
one has seen it coming.
>
> Best Regards,
>
> Boris
>
>>>   };
>>>     static struct clk_lookup usart_clocks_lookups[] = {
>>> --
>>> 1.8.5.2
>>>
>

^ permalink raw reply

* [PATCH v2 2/9] clk: samsung: Provide common helpers for register save/restore
From: Heiko Stübner @ 2014-02-07  9:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391710616-14226-3-git-send-email-t.figa@samsung.com>

Am Donnerstag, 6. Februar 2014, 19:16:49 schrieb Tomasz Figa:
> As suspend/resume handlers are being moved to SoC specific code, due to
> differencies in suspend/resume handling of particular SoCs, to minimize
> code duplication this patch provides common register save/restore
> helpers that save/restore given list of registers of clock controller.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>

Acked-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/clk/samsung/clk.c | 32 ++++++++++++++++++++++++++++++++
>  drivers/clk/samsung/clk.h | 10 ++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index f503f32..c0a716b 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -22,6 +22,38 @@ static struct clk_onecell_data clk_data;
>  #endif
> 
>  #ifdef CONFIG_PM_SLEEP
> +void samsung_clk_save(void __iomem *base,
> +				    struct samsung_clk_reg_dump *rd,
> +				    unsigned int num_regs)
> +{
> +	for (; num_regs > 0; --num_regs, ++rd)
> +		rd->value = readl(base + rd->offset);
> +}
> +
> +void samsung_clk_restore(void __iomem *base,
> +				      const struct samsung_clk_reg_dump *rd,
> +				      unsigned int num_regs)
> +{
> +	for (; num_regs > 0; --num_regs, ++rd)
> +		writel(rd->value, base + rd->offset);
> +}
> +
> +struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(unsigned long
> *rdump, +							unsigned long nr_rdump)
> +{
> +	struct samsung_clk_reg_dump *rd;
> +	unsigned int i;
> +
> +	rd = kcalloc(nr_rdump, sizeof(*rd), GFP_KERNEL);
> +	if (!rd)
> +		return NULL;
> +
> +	for (i = 0; i < nr_rdump; ++i)
> +		rd[i].offset = rdump[i];
> +
> +	return rd;
> +}
> +
>  static struct samsung_clk_reg_dump *reg_dump;
>  static unsigned long nr_reg_dump;
> 
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index 31b4174..ec8d46b 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -340,4 +340,14 @@ extern void __init samsung_clk_register_pll(struct
> samsung_pll_clock *pll_list,
> 
>  extern unsigned long _get_rate(const char *clk_name);
> 
> +extern void samsung_clk_save(void __iomem *base,
> +			     struct samsung_clk_reg_dump *rd,
> +			     unsigned int num_regs);
> +extern void samsung_clk_restore(void __iomem *base,
> +				const struct samsung_clk_reg_dump *rd,
> +				unsigned int num_regs);
> +extern struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> +							unsigned long *rdump,
> +							unsigned long nr_rdump);
> +
>  #endif /* __SAMSUNG_CLK_H */

^ permalink raw reply

* [PATCH 2/2] ARM64: powernv: remove redundant cpuidle_idle_call()
From: Catalin Marinas @ 2014-02-07  9:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391696188-14540-2-git-send-email-nicolas.pitre@linaro.org>

On 6 February 2014 14:16, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> The core idle loop now takes care of it.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* [PATCH v2 7/9] clk: samsung: Drop old suspend/resume code
From: Heiko Stübner @ 2014-02-07  9:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391710616-14226-8-git-send-email-t.figa@samsung.com>

Am Donnerstag, 6. Februar 2014, 19:16:54 schrieb Tomasz Figa:
> Since all SoC drivers have been moved to local suspend/resume handling,
> the old code can be safely dropped.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>

So it looks like I'll need to respin my s3c24xx-ccf patches again, but 
nevertheless:

Acked-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/clk/samsung/clk-exynos4.c    |  2 +-
>  drivers/clk/samsung/clk-exynos5250.c |  2 +-
>  drivers/clk/samsung/clk-exynos5420.c |  2 +-
>  drivers/clk/samsung/clk-exynos5440.c |  2 +-
>  drivers/clk/samsung/clk-s3c64xx.c    |  2 +-
>  drivers/clk/samsung/clk.c            | 54
> +----------------------------------- drivers/clk/samsung/clk.h            |
>  4 +--
>  7 files changed, 7 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos4.c
> b/drivers/clk/samsung/clk-exynos4.c index 325f292..b620a83 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -1122,7 +1122,7 @@ static void __init exynos4_clk_init(struct device_node
> *np, if (!reg_base)
>  		panic("%s: failed to map registers\n", __func__);
> 
> -	samsung_clk_init(np, reg_base, CLK_NR_CLKS, NULL, 0, NULL, 0);
> +	samsung_clk_init(np, reg_base, CLK_NR_CLKS);
> 
>  	samsung_clk_of_register_fixed_ext(exynos4_fixed_rate_ext_clks,
>  			ARRAY_SIZE(exynos4_fixed_rate_ext_clks),
> diff --git a/drivers/clk/samsung/clk-exynos5250.c
> b/drivers/clk/samsung/clk-exynos5250.c index b3cccf0..e7ee442 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -694,7 +694,7 @@ static void __init exynos5250_clk_init(struct
> device_node *np) panic("%s: unable to determine soc\n", __func__);
>  	}
> 
> -	samsung_clk_init(np, reg_base, CLK_NR_CLKS, NULL, 0, NULL, 0);
> +	samsung_clk_init(np, reg_base, CLK_NR_CLKS);
>  	samsung_clk_of_register_fixed_ext(exynos5250_fixed_rate_ext_clks,
>  			ARRAY_SIZE(exynos5250_fixed_rate_ext_clks),
>  			ext_clk_match);
> diff --git a/drivers/clk/samsung/clk-exynos5420.c
> b/drivers/clk/samsung/clk-exynos5420.c index 8ce0780..60b2681 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -786,7 +786,7 @@ static void __init exynos5420_clk_init(struct
> device_node *np) panic("%s: unable to determine soc\n", __func__);
>  	}
> 
> -	samsung_clk_init(np, reg_base, CLK_NR_CLKS, NULL, 0, NULL, 0);
> +	samsung_clk_init(np, reg_base, CLK_NR_CLKS);
>  	samsung_clk_of_register_fixed_ext(exynos5420_fixed_rate_ext_clks,
>  			ARRAY_SIZE(exynos5420_fixed_rate_ext_clks),
>  			ext_clk_match);
> diff --git a/drivers/clk/samsung/clk-exynos5440.c
> b/drivers/clk/samsung/clk-exynos5440.c index cbc15b5..2bfad5a 100644
> --- a/drivers/clk/samsung/clk-exynos5440.c
> +++ b/drivers/clk/samsung/clk-exynos5440.c
> @@ -101,7 +101,7 @@ static void __init exynos5440_clk_init(struct
> device_node *np) return;
>  	}
> 
> -	samsung_clk_init(np, reg_base, CLK_NR_CLKS, NULL, 0, NULL, 0);
> +	samsung_clk_init(np, reg_base, CLK_NR_CLKS);
>  	samsung_clk_of_register_fixed_ext(exynos5440_fixed_rate_ext_clks,
>  		ARRAY_SIZE(exynos5440_fixed_rate_ext_clks), ext_clk_match);
> 
> diff --git a/drivers/clk/samsung/clk-s3c64xx.c
> b/drivers/clk/samsung/clk-s3c64xx.c index d3fbfa5..8bda658 100644
> --- a/drivers/clk/samsung/clk-s3c64xx.c
> +++ b/drivers/clk/samsung/clk-s3c64xx.c
> @@ -465,7 +465,7 @@ void __init s3c64xx_clk_init(struct device_node *np,
> unsigned long xtal_f, panic("%s: failed to map registers\n", __func__);
>  	}
> 
> -	samsung_clk_init(np, reg_base, NR_CLKS, NULL, 0, NULL, 0);
> +	samsung_clk_init(np, reg_base, NR_CLKS);
> 
>  	/* Register external clocks. */
>  	if (!np)
> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index ec761e3..91bec3e 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -21,7 +21,6 @@ static void __iomem *reg_base;
>  static struct clk_onecell_data clk_data;
>  #endif
> 
> -#ifdef CONFIG_PM_SLEEP
>  void samsung_clk_save(void __iomem *base,
>  				    struct samsung_clk_reg_dump *rd,
>  				    unsigned int num_regs)
> @@ -55,63 +54,12 @@ struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> return rd;
>  }
> 
> -static struct samsung_clk_reg_dump *reg_dump;
> -static unsigned long nr_reg_dump;
> -
> -static int samsung_clk_suspend(void)
> -{
> -	struct samsung_clk_reg_dump *rd = reg_dump;
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_reg_dump; i++, rd++)
> -		rd->value = __raw_readl(reg_base + rd->offset);
> -
> -	return 0;
> -}
> -
> -static void samsung_clk_resume(void)
> -{
> -	struct samsung_clk_reg_dump *rd = reg_dump;
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_reg_dump; i++, rd++)
> -		__raw_writel(rd->value, reg_base + rd->offset);
> -}
> -
> -static struct syscore_ops samsung_clk_syscore_ops = {
> -	.suspend	= samsung_clk_suspend,
> -	.resume		= samsung_clk_resume,
> -};
> -#endif /* CONFIG_PM_SLEEP */
> -
>  /* setup the essentials required to support clock lookup using ccf */
>  void __init samsung_clk_init(struct device_node *np, void __iomem *base,
> -		unsigned long nr_clks, unsigned long *rdump,
> -		unsigned long nr_rdump, unsigned long *soc_rdump,
> -		unsigned long nr_soc_rdump)
> +			     unsigned long nr_clks)
>  {
>  	reg_base = base;
> 
> -#ifdef CONFIG_PM_SLEEP
> -	if (rdump && nr_rdump) {
> -		unsigned int idx;
> -		reg_dump = kzalloc(sizeof(struct samsung_clk_reg_dump)
> -				* (nr_rdump + nr_soc_rdump), GFP_KERNEL);
> -		if (!reg_dump) {
> -			pr_err("%s: memory alloc for register dump failed\n",
> -					__func__);
> -			return;
> -		}
> -
> -		for (idx = 0; idx < nr_rdump; idx++)
> -			reg_dump[idx].offset = rdump[idx];
> -		for (idx = 0; idx < nr_soc_rdump; idx++)
> -			reg_dump[nr_rdump + idx].offset = soc_rdump[idx];
> -		nr_reg_dump = nr_rdump + nr_soc_rdump;
> -		register_syscore_ops(&samsung_clk_syscore_ops);
> -	}
> -#endif
> -
>  	clk_table = kzalloc(sizeof(struct clk *) * nr_clks, GFP_KERNEL);
>  	if (!clk_table)
>  		panic("could not allocate clock lookup table\n");
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index 93cb8a0..c7141ba 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -313,9 +313,7 @@ struct samsung_pll_clock {
>  		_lock, _con, _rtable, _alias)
> 
>  extern void __init samsung_clk_init(struct device_node *np, void __iomem
> *base, -		unsigned long nr_clks, unsigned long *rdump,
> -		unsigned long nr_rdump, unsigned long *soc_rdump,
> -		unsigned long nr_soc_rdump);
> +				    unsigned long nr_clks);
>  extern void __init samsung_clk_of_register_fixed_ext(
>  		struct samsung_fixed_rate_clock *fixed_rate_clk,
>  		unsigned int nr_fixed_rate_clk,

^ permalink raw reply

* [PATCH 1/3] clk: rcar-h2: fix sd0/sd1 divisor table
From: Ben Dooks @ 2014-02-07  9:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87y51nbey3.wl%kuninori.morimoto.gx@gmail.com>

On 07/02/14 06:43, Kuninori Morimoto wrote:
> Hi William, Ben, Laurent
>
>>>>>>>    static const struct clk_div_table cpg_sd01_div_table[] = {
>>>>>>> +    {  0,  2 }, {  1,  3 }, {  2,  4 }, {  3,  6 },
>>>>>>> +    {  4,  8 },
>>>>>>>
>>>>>>>        {  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
>>>>>>>        { 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
>
> According to HW team, datasheet is correct.
> Some HW has above un-documented implementation indeed,
> but these are not supported.
> But, 0x0100 (x1/8) on SD0FC/SD1FC is now supported and documented in
> latest datasheet.

Thanks, we do not have a copy of that so cannot comment.

>>>> sdhi0 showed 156MHz output, and it seemed to work. So there is a
>>>> distinct possibility that the sdh clock also supports setting 12
>>>> for a /10
>
> According to HW there,
> SDHFC will be stopped if you set 0xC.

We'll look at re-working this patch and getting it re-sent.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

^ permalink raw reply

* [PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management
From: Tomasz Figa @ 2014-02-07  9:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1389476346-20396-1-git-send-email-tomasz.figa@gmail.com>

Hi Chris,

On 11.01.2014 22:39, Tomasz Figa wrote:
> On platforms prior to Exynos the SDHCI block used internal clock
> divider controlled by SELFREQ field of CLKCON register to divide base
> clock selected from several external clocks fed to the block by
> SELBASECLK bitfield of CONTROL2 register. Depending on wanted clock
> frequency, different external clock may be the best choice and so
> the driver needs to switch the SELBASECLK mux on the fly.
>
> However the selection logic has been broken for quite some time leaving
> the controller using always clock 0, which is not always the right
> source and leading to suboptimal performance of the SDHCI block on
> affected platforms.
>
> This series intends to fix the problems mentioned above and also clean-up
> clock management code slightly.
>
> Tested on S3C6410-based Mini6410 board, with following performance
> figures:
>
> * Before this series (133 MHz HCLK always selected, leading to at most
>    33 MHz card clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  44 MB in  3.11 seconds =  14.14 MB/sec
> root at tiny6410:~#
>
> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>    clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  60 MB in  3.06 seconds =  19.63 MB/sec
> root at tiny6410:~#
>
> Tomasz Figa (6):
>    mmc: sdhci-s3c: Use shifts to divide by powers of two
>    mmc: sdhci-s3c: Cache bus clock rates
>    mmc: sdhci-s3c: Use correct condition to check for clock presence
>    mmc: sdhci-s3c: Simplify min/max clock calculation
>    mmc: sdhci-s3c: Fix handling of bus clock switching
>    mmc: sdhci-s3c: Do not allow frequencies higher than requested
>
>   drivers/mmc/host/sdhci-s3c.c | 170 ++++++++++++++++++++-----------------------
>   1 file changed, 77 insertions(+), 93 deletions(-)
>

What do you think about this series?

Best regards,
Tomasz

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox