Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v13 7/8] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer
From: Fu Wei @ 2016-09-15 10:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473883781-9571-8-git-send-email-fu.wei@linaro.org>

HI

On 15 September 2016 at 04:09,  <fu.wei@linaro.org> wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The patch add memory-mapped timer register support by using the information
> provided by the new GTDT driver of ACPI.
> Also refactor original memory-mapped timer dt support for reusing some common
> code.
>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 238 +++++++++++++++++++++++++----------
>  1 file changed, 172 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 0197ef9..ddca6e3 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -57,6 +57,7 @@
>  static unsigned arch_timers_present __initdata;
>
>  static void __iomem *arch_counter_base;
> +static void __iomem *cntctlbase __initdata;
>
>  struct arch_timer {
>         void __iomem *base;
> @@ -656,15 +657,56 @@ out:
>         return err;
>  }
>
> -static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
> +static int __init arch_timer_mem_register(struct device_node *np, void *frame)
>  {
> -       int ret;
> -       irq_handler_t func;
> +       struct device_node *frame_node = NULL;
> +       struct gt_timer_data *frame_data = NULL;
>         struct arch_timer *t;
> +       void __iomem *base;
> +       irq_handler_t func;
> +       unsigned int irq;
> +       int ret;
> +
> +       if (np) {
> +               frame_node = (struct device_node *)frame;
> +               base = of_iomap(frame_node, 0);
> +               arch_timer_detect_rate(base, np);
> +               if (arch_timer_mem_use_virtual)
> +                       irq = irq_of_parse_and_map(frame_node, VIRT_SPI);
> +               else
> +                       irq = irq_of_parse_and_map(frame_node, PHYS_SPI);
> +       } else {
> +               frame_data = (struct gt_timer_data *)frame;
> +               /*
> +                * According to ARMv8 Architecture Reference Manual(ARM),
> +                * the size of CNTBaseN frames of memory-mapped timer
> +                * is SZ_4K(Offset 0x000 ? 0xFFF).
> +                */
> +               base = ioremap(frame_data->cntbase_phy, SZ_4K);
> +               if (arch_timer_mem_use_virtual)
> +                       irq = frame_data->virtual_irq;
> +               else
> +                       irq = frame_data->irq;
> +       }
> +
> +       if (!base) {
> +               pr_err("Can't map frame's registers\n");
> +               return -ENXIO;
> +       }
> +       if (!irq) {
> +               pr_err("Frame missing %s irq",
> +                      arch_timer_mem_use_virtual ? "virt" : "phys");
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +
> +       arch_counter_base = base;
>
>         t = kzalloc(sizeof(*t), GFP_KERNEL);
> -       if (!t)
> +       if (!t) {
>                 return -ENOMEM;
                   ^
                  my bad, it should be "ret = "

> +               goto out;
> +       }
>
>         t->base = base;
>         t->evt.irq = irq;
> @@ -676,11 +718,13 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
>                 func = arch_timer_handler_phys_mem;
>
>         ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &t->evt);
> -       if (ret) {
> -               pr_err("Failed to request mem timer irq\n");
> -               kfree(t);
> -       }
> +       if (!ret)
> +               return 0;
>
> +       pr_err("Failed to request mem timer irq\n");
> +       kfree(t);
> +out:
> +       iounmap(base);
>         return ret;
>  }
>
> @@ -769,7 +813,7 @@ static int __init arch_timer_init(void)
>                 return ret;
>
>         arch_timer_kvm_info.virtual_irq = arch_timer_ppi[VIRT_PPI];
> -
> +
>         return 0;
>  }
>
> @@ -803,21 +847,56 @@ static int __init arch_timer_of_init(struct device_node *np)
>  CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
>  CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
>
> -static int __init arch_timer_mem_init(struct device_node *np)
> +static int __init get_cnttidr(struct device_node *np,
> +                             struct gt_block_data *gt_block, u32 *cnttidr)
>  {
> -       struct device_node *frame, *best_frame = NULL;
> -       void __iomem *cntctlbase, *base;
> -       unsigned int irq, ret = -EINVAL;
> -       u32 cnttidr;
> +       if (np)
> +               cntctlbase = of_iomap(np, 0);
> +       else
> +               cntctlbase = ioremap(gt_block->cntctlbase_phy, SZ_4K);
> +               /*
> +                * According to ARMv8 Architecture Reference Manual(ARM),
> +                * the size of CNTCTLBase frame of memory-mapped timer
> +                * is SZ_4K(Offset 0x000 ? 0xFFF).
> +                */
>
> -       arch_timers_present |= ARCH_MEM_TIMER;
> -       cntctlbase = of_iomap(np, 0);
>         if (!cntctlbase) {
>                 pr_err("Can't find CNTCTLBase\n");
>                 return -ENXIO;
>         }
>
> -       cnttidr = readl_relaxed(cntctlbase + CNTTIDR);
> +       *cnttidr = readl_relaxed(cntctlbase + CNTTIDR);
> +       return 0;
> +}
> +
> +static bool __init is_best_frame(u32 cnttidr, int n)
> +{
> +       u32 cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT | CNTACR_RWVT |
> +                    CNTACR_RVOFF | CNTACR_RVCT;
> +
> +       /* Try enabling everything, and see what sticks */
> +       writel_relaxed(cntacr, cntctlbase + CNTACR(n));
> +       cntacr = readl_relaxed(cntctlbase + CNTACR(n));
> +
> +       if ((cnttidr & CNTTIDR_VIRT(n)) &&
> +           !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT)))
> +               arch_timer_mem_use_virtual = true;
> +       else if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> +               return false;
> +
> +       return true;
> +}
> +
> +static int __init arch_timer_mem_init(struct device_node *np)
> +{
> +       struct device_node *frame, *best_frame = NULL;
> +       unsigned int ret = -EINVAL;
> +       u32 cnttidr;
> +
> +       arch_timers_present |= ARCH_MEM_TIMER;
> +
> +       if (get_cnttidr(np, NULL, &cnttidr))
> +               return -ENXIO;
>
>         /*
>          * Try to find a virtual capable frame. Otherwise fall back to a
> @@ -825,60 +904,22 @@ static int __init arch_timer_mem_init(struct device_node *np)
>          */
>         for_each_available_child_of_node(np, frame) {
>                 int n;
> -               u32 cntacr;
> -
>                 if (of_property_read_u32(frame, "frame-number", &n)) {
> -                       pr_err("Missing frame-number\n");
> +                       pr_err("Missing frame-number.\n");
>                         of_node_put(frame);
>                         goto out;
>                 }
> -
> -               /* Try enabling everything, and see what sticks */
> -               cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT |
> -                        CNTACR_RWVT | CNTACR_RVOFF | CNTACR_RVCT;
> -               writel_relaxed(cntacr, cntctlbase + CNTACR(n));
> -               cntacr = readl_relaxed(cntctlbase + CNTACR(n));
> -
> -               if ((cnttidr & CNTTIDR_VIRT(n)) &&
> -                   !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT))) {
> +               if (is_best_frame(cnttidr, n)) {
>                         of_node_put(best_frame);
> -                       best_frame = frame;
> -                       arch_timer_mem_use_virtual = true;
> -                       break;
> +                       best_frame = of_node_get(frame);
> +                       if (arch_timer_mem_use_virtual)
> +                               break;
>                 }
> -
> -               if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
> -                       continue;
> -
> -               of_node_put(best_frame);
> -               best_frame = of_node_get(frame);
> -       }
> -
> -       ret= -ENXIO;
> -       base = arch_counter_base = of_iomap(best_frame, 0);
> -       if (!base) {
> -               pr_err("Can't map frame's registers\n");
> -               goto out;
> -       }
> -
> -       if (arch_timer_mem_use_virtual)
> -               irq = irq_of_parse_and_map(best_frame, VIRT_SPI);
> -       else
> -               irq = irq_of_parse_and_map(best_frame, PHYS_SPI);
> -
> -       ret = -EINVAL;
> -       if (!irq) {
> -               pr_err("Frame missing %s irq",
> -                      arch_timer_mem_use_virtual ? "virt" : "phys");
> -               goto out;
>         }
>
> -       arch_timer_detect_rate(base, np);
> -       ret = arch_timer_mem_register(base, irq);
> -       if (ret)
> -               goto out;
> -
> -       return arch_timer_common_init();
> +       ret = arch_timer_mem_register(np, best_frame);
> +       if (!ret)
> +               ret = arch_timer_common_init();
>  out:
>         iounmap(cntctlbase);
>         of_node_put(best_frame);
> @@ -888,7 +929,72 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
>                        arch_timer_mem_init);
>
>  #ifdef CONFIG_ACPI_GTDT
> -/* Initialize per-processor generic timer */
> +static struct gt_timer_data __init *arch_timer_mem_get_timer(
> +                                               struct gt_block_data *gt_blocks)
> +{
> +       struct gt_block_data *gt_block = gt_blocks;
> +       struct gt_timer_data *best_frame = NULL;
> +       u32 cnttidr;
> +       int i;
> +
> +       if (get_cnttidr(NULL, gt_block, &cnttidr))
> +               return NULL;
> +       /*
> +        * Try to find a virtual capable frame. Otherwise fall back to a
> +        * physical capable frame.
> +        */
> +       for (i = 0; i < gt_block->timer_count; i++) {
> +               if (is_best_frame(cnttidr, gt_block->timer[i].frame_nr)) {
> +                       best_frame = &gt_block->timer[i];
> +                       if (arch_timer_mem_use_virtual)
> +                               break;
> +               }
> +       }
> +       iounmap(cntctlbase);
> +
> +       return best_frame;
> +}
> +
> +static int __init arch_timer_mem_acpi_init(size_t timer_count)
> +{
> +       struct gt_block_data *gt_blocks;
> +       struct gt_timer_data *gt_timer;
> +       int ret = -EINVAL;
> +
> +       /*
> +        * If we don't have any Platform Timer Structures, just return.
> +        */
> +       if (!timer_count)
> +               return 0;
> +
> +       /*
> +        * before really check all the Platform Timer Structures,
> +        * we assume they are GT block, and allocate memory for them.
> +        * We will free these memory once we finish the initialization.
> +        */
> +       gt_blocks = kcalloc(timer_count, sizeof(*gt_blocks), GFP_KERNEL);
> +       if (!gt_blocks)
> +               return -ENOMEM;
> +
> +       if (gtdt_arch_timer_mem_init(gt_blocks) > 0) {
> +               gt_timer = arch_timer_mem_get_timer(gt_blocks);
> +               if (!gt_timer) {
> +                       pr_err("Failed to get mem timer info.\n");
> +                       goto error;
> +               }
> +               ret = arch_timer_mem_register(NULL, gt_timer);
> +               if (ret) {
> +                       pr_err("Failed to register mem timer.\n");
> +                       goto error;
> +               }
> +       }
> +       arch_timers_present |= ARCH_MEM_TIMER;
> +error:
> +       kfree(gt_blocks);
> +       return ret;
> +}
> +
> +/* Initialize per-processor generic timer and memory-mapped timer(if present) */
>  static int __init arch_timer_acpi_init(struct acpi_table_header *table)
>  {
>         int timer_count;
> @@ -912,8 +1018,8 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
>         /* Get the frequency from CNTFRQ */
>         arch_timer_detect_rate(NULL, NULL);
>
> -       if (timer_count < 0)
> -               pr_err("Failed to get platform timer info.\n");
> +       if (timer_count < 0 || arch_timer_mem_acpi_init((size_t)timer_count))
> +               pr_err("Failed to initialize memory-mapped timer.\n");
>
>         return arch_timer_init();
>  }
> --
> 2.7.4
>



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Lorenzo Pieralisi @ 2016-09-15 10:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57D7E53F.1000106@huawei.com>

On Tue, Sep 13, 2016 at 07:38:39PM +0800, Dongdong Liu wrote:

[...]

> Our host bridge is non ECAM only for the RC bus config space;
> for any other bus underneath the root bus we support ECAM access.
> 
> RC config resource with hardcode as DEFINE_RES_MEM(0xb0070000, SZ_4K),
> EP config resource we get it from MCFG table.
> So we need to override ops, but config resource we only need to hardcode with RC config resource.
> 
> Our host controller ACPI support patch can be found:
> https://lkml.org/lkml/2016/8/31/340

Sorry I misread your code. IIUC you create an array of resources that
represent non-ECAM config space (and incidentally contain debug
registers to check the link status - that you need to check for every
given config access (?)), but you still need to have an MCFG entry that
covers the bus number subject to quirk to make sure this mechanism
works. Correct ?

This also means that, with the MCFG tables you have and current
mainline kernel you are able to probe a root bridge (because the MCFG
table covers the bus number that is not ECAM), with enumeration
going haywire because it is trying to carry out ECAM accesses on
non-ECAM space.

Is my reading correct ?

Anyway, that's not stricly related to this discussion, it is time we
converge on this patchset, we can add a domain range if that
simplifies things.

Thanks,
Lorenzo

> This patch is based on RFC V5 quirk mechanism.
> 
> Based on V6 quirk mechanism, we have to change it as below:
> 
> #ifdef CONFIG_PCI_HISI_ACPI
> 	{ "HISI ", "HIP05 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP05 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP05 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP05 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP06 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP06 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP06 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP06 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
>          MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP07 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> 	  MCFG_RES_EMPTY},
> 	{ "HISI ", "HIP07 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> 	  MCFG_RES_EMPTY},
> 	....
> 	
> 	{ "HISI ", "HIP07 ", 0, 15, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> 	  MCFG_RES_EMPTY},
> 
> #endif
> 
> struct pci_ecam_ops hisi_pci_hip05_ops = {
> 	.bus_shift	= 20,
> 	.init		=  hisi_pci_hip05_init,
> 	.pci_ops	= {
> 		.map_bus	= pci_ecam_map_bus,
> 		.read		= hisi_pcie_acpi_rd_conf,
> 		.write		= hisi_pcie_acpi_wr_conf,
> 	}
> };
> 
> struct pci_ecam_ops hisi_pci_hip06_ops = {
> 	.bus_shift = 20,
> 	.init = hisi_pci_hip06_init,
> 	.pci_ops = {
> 		.map_bus = pci_ecam_map_bus,
> 		.read = hisi_pcie_acpi_rd_conf,
> 		.write = hisi_pcie_acpi_wr_conf,
> 	}
> };
> 
> hisi_pci_hipxx_init function is used to get RC config resource with hardcode.
> .....
> 
> So I hope we can use MCFG_DOM_RANGE, Then I can change it as below.
> 
> #ifdef CONFIG_PCI_HISI_ACPI
> 	{ "HISI  ", "HIP05   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> 	 &hisi_pcie_hip05_ops, MCFG_RES_EMPTY},
> 	{ "HISI  ", "HIP06   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> 	 &hisi_pcie_hip06_ops, MCFG_RES_EMPTY},
> 	{ "HISI  ", "HIP07   ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY,
> 	  &hisi_pcie_hip07_ops, MCFG_RES_EMPTY},
> #endif
> 
> Thanks
> Dongdong
> >
> >Thanks,
> >Tomasz
> >
> >.
> >
> 

^ permalink raw reply

* [GIT PULL] Greybus driver subsystem for 4.9-rc1
From: Bryan O'Donoghue @ 2016-09-15 11:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915101741.GA21567@kroah.com>

On Thu, 2016-09-15 at 12:17 +0200, Greg KH wrote:
> This
> > is the first I've heard of timesync having a binding. I can't
> imagine
> > why it needs one.
> 
> Ah, I'll let Bryan answer that one :)

It's possible we could drop the binding. It was needed to describe the
register location of the MMIO architectural register on MSM8994. I have
this binding ATM the describe fact that get_cycles() on MSM8994 returns
a free-running counter, clocked by refclk and that refclk is provided
to each processor that want to do FrameTime - i.e. the clock driving
get_cycles() comes from PMIC and drives the relevant PLLs on the
downstream processors clocking their respective TMR blocks.

static const struct of_device_id arch_timer_of_match[] = {
????????{ .compatible???= "google,greybus-frame-time-counter", },
????????{},
};

I'm not aware of a corresponding kernel API that describes the
frequency get_cycles() operates at but if there is one then there's no
need for this binding.

^ permalink raw reply

* [GIT PULL] Greybus driver subsystem for 4.9-rc1
From: Mark Rutland @ 2016-09-15 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473935756.10230.42.camel@nexus-software.ie>

Hi,

More questions below. Perhaps some of these will be implicitly answered
when the linearised patches appear, and I'm happy to wait until then to
continue the discussion, as I suspect otherwise we're all likely to end
up exasperated.

Please do Cc me on those.

Regardless, until those appear and a reasonable time has been given for
replies, my comments regarding the lack of review stand, as does my NAK
for the series.

On Thu, Sep 15, 2016 at 11:35:56AM +0100, Bryan O'Donoghue wrote:
> On Thu, 2016-09-15 at 11:13 +0100, Mark Rutland wrote:
> > On Thu, Sep 15, 2016 at 10:35:33AM +0100, Bryan O'Donoghue wrote:
> > >?
> > I don't think the history matters, 
> 
> Your comment seemed to indicate you thought we were reading a
> architectural timer directly - which we aren't.

Sure, and as I pointed out, the comment in the HEAD commit still claims
it does, even if the code doesn't. This is at best, confusing, and the
history of how it came to be there doesn't really matter...

> > and I don't think that one can rely
> > on get_cycles() in this manner outside of arch code.
> 
> I don't follow your meaning. What's wrong with get_cycles() ? You've
> already said you don't think reading an architectural timer directly is
> correct.

I pointed out a number of issues in my previous reply.

For example, you have absolutely no guarantee as to what backs
get_cycles(). Despite this, the code assumes that get_cycles() is backed
by something running at the frequency described in a
"google,greybus-frame-time-counter" node.

Even if this *happens* to match what some piece of arch code provides
today on some platform, it is in no way *guaranteed*.

> The objective is to read one of the free-running counters in MSM8994,
> clocked by the PMIC. The refclk provided by PMIC is distributed to each
> processor in the system.
> 
> >  Looking at the
> > state of the tree [1] as of the final commit [2] in the greybus
> > branch,
> > my points still stand:
> > 
> > * The "google,greybus-frame-time-counter" node is superfluous. It
> > does
> > ? not describe a particular device,
> 
> It describes a timer running @ 19.2MHz, clocked by PMIC refclk.

... which you assume is whatever backs get_cycles(), which you in
practice assume is the architected timer. For which we *already* have a
binding and driver.

Given that, as far as I can tell, "google,greybus-frame-time-counter"
describes a software construct that uses this, not an actual piece of
hardware.

> >  and duplicates information we have ? elsewhere.
> 
> Can you give an example ?

Trivially, the CNTFRQ register in the architected timer (which is common
across MMIO/sysreg), which you can query with arch_timer_get_rate().

Note that isn't guaranteed to match get_cycles() either. You need a
better API to call.

> > * The clock-frequency property isn't necessary. The architected timer
> > ? drivers know the frequency of the architected timers (MMIO or
> > sysreg),
> > ? and they should be queried as to the frequency.
> 
> OK so if I'm understanding you. You think get_cycles() is fine but that
> instead of encoding a "greybus-frame-time-counter" the platform code
> should interrogate the frequency provided - is that correct ?

You should definitely interrogate the relevant driver, somehow.

Without a higher-level view of what you're trying to achieve, it's not
clear to me whether get_cycles() is the right interface.

> > Beyond that, the fallback code using cpufreq and presumably an actual
> > cycle counter will be broken in a number of cases
> 
> Of course the fallback will be broken... it's not supposed to work if
> you don't have a timer that can be used - just compile, run and print a
> complaint - i.e., this won't really do FrameTime on an x86... then
> again since so much of the underlying greybus/unipro hardware -
> requires a 19.2MHz refclk - if you were to try to do greybus on x86
> you'd need to solve that problem.

If it's never going to work, why give the illusion that it might?

If you don't have the necessary prerequisites, fail to probe entirely.
Prevent drivers that depend on the non-existent functionality from
probing, or have them avoid the facility which is not available.

Don't provide them with something that can appear to work for a while,
yet will fall over in a breeze.

> > Per the comment at the top of the file, it looks like you want a
> > system-wide stable clocksource. If you want that, you need to use a
> > generic API that allows drivers and arch code to actually provide
> > that, rather than building one yourself that doesn't work.
> 
> Hmm. The objective is to read one of the timers clocked by the PMIC
> refclk input. refclk is provided to each processor in the system - and
> on MSM8994 clocks the MMIO timers. It's used to drive the PLL on the
> other processors - which in turn drive the timers that the Modules use
> to read their own local counters. We want to read that counter on MSM
> directly - get_cycles() has worked nicely so far.

This is too low-level for me to see what you're actually trying to
achieve. The fact that you want to read a specific timer is an
implementation detail.

Why can't you use any other timer? Correctness? Performance? (Why) is
the fact that the PLL drives module timers important?

> > If you're trying to synchronise with other agents in the system that
> > are reading from the MMIO arch timers,
> 
> No. The MMIO timers are useful only to the MSM. We don't have any type
> of parallel (or serial) bus that can access that on-chip resource.

To be clear, by "other agents in the system", I'm also asking about
other devices within the SoC (e.g. anything other than the CPUs running
this instance of Linux).

> MSM8994 -- > USB
> ? ? ? ? ? ? ?APBridge (timer) -> UniPro bus
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -> Module with a UART
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -> Module with a GPIO
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -> Module with an etc, etc
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -> SPI bus
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -> SVC
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Owns FrameTime
> 
> So the SVC owns FrameTime and diseminates that to other entities in the
> system by way of a GPIO and greybus. It's up to the MSM8994 to select a
> timer that works for it - the other processors in the system are
> responsible for their own timers.

Sorry, but this doesn't clarify much from my PoV.

* What are the requirements for that timer? 

* Is there_any_ implicit relationship with the module timers derived
  from the fact they share a parent PLL? Is that a requirement somehow?

Thanks,
Mark.

^ permalink raw reply

* [PATCH] coresight: tmc: Cleanup operation mode handling
From: Venkatesh Vivekanandan @ 2016-09-15 11:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473861230-5666-1-git-send-email-suzuki.poulose@arm.com>

On Wed, Sep 14, 2016 at 7:23 PM, Suzuki K Poulose
<suzuki.poulose@arm.com> wrote:
> The mode of operation of the TMC tracked in drvdata->mode is defined
> as a local_t type. This is always checked and modified under the
> drvdata->spinlock and hence we don't need local_t for it and the
> unnecessary synchronisation instructions that comes with it. This
> change makes the code a bit more cleaner.
>
> Also fixes the order in which we update the drvdata->mode to
> CS_MODE_DISABLED. i.e, in tmc_disable_etX_sink we change the
> mode to CS_MODE_DISABLED before invoking tmc_disable_etX_hw()
> which in turn depends on the mode to decide whether to dump the
> trace to a buffer.
>
> Applies on mathieu's coresight/next tree [1]
>
> https://git.linaro.org/kernel/coresight.git next
>
> Reported-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etf.c | 32 +++++++++++--------------
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 30 ++++++++++-------------
>  drivers/hwtracing/coresight/coresight-tmc.h     |  2 +-
>  3 files changed, 28 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index d6941ea..c51ce45 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -70,7 +70,7 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>          * When operating in sysFS mode the content of the buffer needs to be
>          * read before the TMC is disabled.
>          */
> -       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 tmc_etb_dump_hw(drvdata);
>         tmc_disable_hw(drvdata);
>
> @@ -108,7 +108,6 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
>         int ret = 0;
>         bool used = false;
>         char *buf = NULL;
> -       long val;
>         unsigned long flags;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -138,13 +137,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
>                 goto out;
>         }
>
> -       val = local_xchg(&drvdata->mode, mode);
>         /*
>          * In sysFS mode we can have multiple writers per sink.  Since this
>          * sink is already enabled no memory is needed and the HW need not be
>          * touched.
>          */
> -       if (val == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 goto out;
>
>         /*
> @@ -163,6 +161,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev, u32 mode)
>                 drvdata->buf = buf;
>         }
>
> +       drvdata->mode = CS_MODE_SYSFS;
>         tmc_etb_enable_hw(drvdata);
>  out:
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -180,7 +179,6 @@ out:
>  static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
>  {
>         int ret = 0;
> -       long val;
>         unsigned long flags;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -194,17 +192,17 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, u32 mode)
>                 goto out;
>         }
>
> -       val = local_xchg(&drvdata->mode, mode);
>         /*
>          * In Perf mode there can be only one writer per sink.  There
>          * is also no need to continue if the ETB/ETR is already operated
>          * from sysFS.
>          */
> -       if (val != CS_MODE_DISABLED) {
> +       if (drvdata->mode != CS_MODE_DISABLED) {
>                 ret = -EINVAL;
>                 goto out;
>         }
>
> +       drvdata->mode = mode;
>         tmc_etb_enable_hw(drvdata);
>  out:
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -227,7 +225,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>
>  static void tmc_disable_etf_sink(struct coresight_device *csdev)
>  {
> -       long val;
>         unsigned long flags;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -237,10 +234,11 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
>                 return;
>         }
>
> -       val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>         /* Disable the TMC only if it needs to */
> -       if (val != CS_MODE_DISABLED)
> +       if (drvdata->mode != CS_MODE_DISABLED) {
>                 tmc_etb_disable_hw(drvdata);
> +               drvdata->mode = CS_MODE_DISABLED;
> +       }
>
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -260,7 +258,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
>         }
>
>         tmc_etf_enable_hw(drvdata);
> -       local_set(&drvdata->mode, CS_MODE_SYSFS);
> +       drvdata->mode = CS_MODE_SYSFS;
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
>         dev_info(drvdata->dev, "TMC-ETF enabled\n");
> @@ -279,8 +277,8 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
>                 return;
>         }
>
> +       drvdata->mode = CS_MODE_DISABLED;
>         tmc_etf_disable_hw(drvdata);
> -       local_set(&drvdata->mode, CS_MODE_DISABLED);
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
>         dev_info(drvdata->dev, "TMC disabled\n");
> @@ -383,7 +381,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>                 return;
>
>         /* This shouldn't happen */
> -       if (WARN_ON_ONCE(local_read(&drvdata->mode) != CS_MODE_PERF))
> +       if (WARN_ON_ONCE(drvdata->mode != CS_MODE_PERF))
>                 return;
>
>         CS_UNLOCK(drvdata->base);
> @@ -504,7 +502,6 @@ const struct coresight_ops tmc_etf_cs_ops = {
>
>  int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
>  {
> -       long val;
>         enum tmc_mode mode;
>         int ret = 0;
>         unsigned long flags;
> @@ -528,9 +525,8 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
>                 goto out;
>         }
>
> -       val = local_read(&drvdata->mode);
>         /* Don't interfere if operated from Perf */
> -       if (val == CS_MODE_PERF) {
> +       if (drvdata->mode == CS_MODE_PERF) {
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -542,7 +538,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
>         }
>
>         /* Disable the TMC if need be */
> -       if (val == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 tmc_etb_disable_hw(drvdata);
>
>         drvdata->reading = true;
> @@ -573,7 +569,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
>         }
>
>         /* Re-enable the TMC if need be */
> -       if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
> +       if (drvdata->mode == CS_MODE_SYSFS) {
>                 /*
>                  * The trace run will continue with the same allocated trace
>                  * buffer. As such zero-out the buffer so that we don't end
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 886ea83..cf2bf60 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -86,7 +86,7 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>          * When operating in sysFS mode the content of the buffer needs to be
>          * read before the TMC is disabled.
>          */
> -       if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 tmc_etr_dump_hw(drvdata);
>         tmc_disable_hw(drvdata);
>
> @@ -97,7 +97,6 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
>  {
>         int ret = 0;
>         bool used = false;
> -       long val;
>         unsigned long flags;
>         void __iomem *vaddr = NULL;
>         dma_addr_t paddr;
> @@ -134,13 +133,12 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
>                 goto out;
>         }
>
> -       val = local_xchg(&drvdata->mode, mode);
>         /*
>          * In sysFS mode we can have multiple writers per sink.  Since this
>          * sink is already enabled no memory is needed and the HW need not be
>          * touched.
>          */
> -       if (val == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 goto out;
>
>         /*
> @@ -157,6 +155,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev, u32 mode)
>
>         memset(drvdata->vaddr, 0, drvdata->size);
>
> +       drvdata->mode = CS_MODE_SYSFS;
>         tmc_etr_enable_hw(drvdata);
>  out:
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -174,7 +173,6 @@ out:
>  static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
>  {
>         int ret = 0;
> -       long val;
>         unsigned long flags;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -188,18 +186,18 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, u32 mode)
>                 goto out;
>         }
>
> -       val = local_xchg(&drvdata->mode, mode);
>         /*
>          * In Perf mode there can be only one writer per sink.  There
>          * is also no need to continue if the ETR is already operated
>          * from sysFS.
>          */
> -       if (val != CS_MODE_DISABLED) {
> +       if (drvdata->mode == CS_MODE_DISABLED) {
> +               drvdata->mode = CS_MODE_PERF;
> +               tmc_etr_enable_hw(drvdata);
> +       } else {
>                 ret = -EINVAL;
> -               goto out;
>         }
>
> -       tmc_etr_enable_hw(drvdata);
>  out:
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -221,7 +219,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
>
>  static void tmc_disable_etr_sink(struct coresight_device *csdev)
>  {
> -       long val;
>         unsigned long flags;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -231,10 +228,11 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
>                 return;
>         }
>
> -       val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>         /* Disable the TMC only if it needs to */
> -       if (val != CS_MODE_DISABLED)
> +       if (drvdata->mode != CS_MODE_DISABLED) {
>                 tmc_etr_disable_hw(drvdata);
> +               drvdata->mode = CS_MODE_DISABLED;
> +       }
>
>         spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> @@ -253,7 +251,6 @@ const struct coresight_ops tmc_etr_cs_ops = {
>  int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>  {
>         int ret = 0;
> -       long val;
>         unsigned long flags;
>
>         /* config types are set a boot time and never change */
> @@ -266,9 +263,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>                 goto out;
>         }
>
> -       val = local_read(&drvdata->mode);
>         /* Don't interfere if operated from Perf */
> -       if (val == CS_MODE_PERF) {
> +       if (drvdata->mode == CS_MODE_PERF) {
>                 ret = -EINVAL;
>                 goto out;
>         }
> @@ -280,7 +276,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>         }
>
>         /* Disable the TMC if need be */
> -       if (val == CS_MODE_SYSFS)
> +       if (drvdata->mode == CS_MODE_SYSFS)
>                 tmc_etr_disable_hw(drvdata);
>
>         drvdata->reading = true;
> @@ -303,7 +299,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
>         spin_lock_irqsave(&drvdata->spinlock, flags);
>
>         /* RE-enable the TMC if need be */
> -       if (local_read(&drvdata->mode) == CS_MODE_SYSFS) {
> +       if (drvdata->mode == CS_MODE_SYSFS) {
>                 /*
>                  * The trace run will continue with the same allocated trace
>                  * buffer. The trace buffer is cleared in tmc_etr_enable_hw(),
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 44b3ae3..51c0185 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -117,7 +117,7 @@ struct tmc_drvdata {
>         void __iomem            *vaddr;
>         u32                     size;
>         u32                     len;
> -       local_t                 mode;
> +       u32                     mode;
>         enum tmc_config_type    config_type;
>         enum tmc_mem_intf_width memwidth;
>         u32                     trigger_cntr;
> --
> 2.7.4
>
tmc_etb_dump_hw is called to dump trace from hardware in ETF mode.

Tested-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>

^ permalink raw reply

* [GIT PULL] arm64: X-Gene platforms DTS changes queued for 4.9 - part1
From: Mark Rutland @ 2016-09-15 11:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915101535.GC3380@arm.com>

On Thu, Sep 15, 2016 at 11:15:35AM +0100, Will Deacon wrote:
> From 436d3a7ad25517433fc8f563f44fdab3c4801666 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will.deacon@arm.com>
> Date: Thu, 15 Sep 2016 10:14:41 +0100
> Subject: [PATCH] MAINTAINERS: Update ARM PMU PROFILING AND DEBUGGING entry
> 
> There are an increasing number of ARM SoC PMU drivers appearing for
> things like interconnects, memory controllers and cache controllers.
> Rather than have these handled on an ad-hoc basis, where SoC maintainers
> each send their PMU drivers directly to arm-soc, let's take these into
> drivers/perf/ and send a single pull request to arm-soc instead, much
> like other subsystems.
> 
> This patch amends the ARM PMU MAINTAINERS entry to include all of
> drivers/perf/ (currently just the ARM CPU PMU), changes Mark Rutland
> from Reviewer to Maintainer, so that he can help with the new tree and
> adds the device-tree binding to the list of maintained files.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  MAINTAINERS | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6781a3febd59..090694e685ba 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -913,15 +913,16 @@ F:	arch/arm/include/asm/floppy.h
>  
>  ARM PMU PROFILING AND DEBUGGING
>  M:	Will Deacon <will.deacon@arm.com>
> -R:	Mark Rutland <mark.rutland@arm.com>
> +M:	Mark Rutland <mark.rutland@arm.com>
>  S:	Maintained
>  F:	arch/arm*/kernel/perf_*
>  F:	arch/arm/oprofile/common.c
>  F:	arch/arm*/kernel/hw_breakpoint.c
>  F:	arch/arm*/include/asm/hw_breakpoint.h
>  F:	arch/arm*/include/asm/perf_event.h
> -F:	drivers/perf/arm_pmu.c
> +F:	drivers/perf/*
>  F:	include/linux/perf/arm_pmu.h
> +F:	Documentation/devicetree/bindings/arm/pmu.txt
>  
>  ARM PORT
>  M:	Russell King <linux@armlinux.org.uk>
> -- 
> 2.5.0
> 

^ permalink raw reply

* [PATCH 1/7] mfd: max8997-irq: 'inline' should be at the beginning of the declaration
From: Arnd Bergmann @ 2016-09-15 11:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915104521.14286-2-lee.jones@linaro.org>

On Thursday, September 15, 2016 11:45:15 AM CEST Lee Jones wrote:
> -static const inline struct max8997_irq_data *
> +inline static const struct max8997_irq_data *

I think this causes another warning, the order should be
"stastic inline const struct max8997_irq_data".

	Arnd

^ permalink raw reply

* [PATCH 2/7] mfd: ab8500-debugfs: Prevent initialised field from being over-written
From: Arnd Bergmann @ 2016-09-15 11:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915104521.14286-3-lee.jones@linaro.org>

On Thursday, September 15, 2016 11:45:16 AM CEST Lee Jones wrote:
> Due to the lack of parity in the way array fields have been named/
> numbered, a mistake was made where more debug fields were declared
> than actually existed.  In doing so, 2 fields were added, which
> although unclear, were already declared in the array.  The result
> was that the latter declarations trashed the former ones.
> 
> This patch places the array back in the correct order and removes
> the offending NULL entries.
> 
> While we're at it, let's ensure this doesn't happen again by naming
> each field properly and add a new *_LAST define to describe how
> many fields there should be.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/ab8500-debugfs.c      | 50 ++++++++++++++++-----------------------
>  include/linux/mfd/abx500/ab8500.h |  2 ++
>  2 files changed, 23 insertions(+), 29 deletions(-)

I tried doing the same thing at some point and never submitted.
For reference, here is my version. Yours looks better though,
so keep that.

	Arnd

commit daa445be130176d946af78e7a8460e44d3a792dd
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Wed Jun 8 10:29:18 2016 +0200

    mfd: ab8500: remove duplicate table entries
    
    gcc -Wextra finds two duplicate initializations for ab8500_debug_ranges:
    
    drivers/mfd/ab8500-debugfs.c:466:11: error: initialized field overwritten [-Werror=override-init]
    drivers/mfd/ab8500-debugfs.c:466:11: note: (near initialization for 'ab8500_debug_ranges[17]')
    drivers/mfd/ab8500-debugfs.c:470:11: error: initialized field overwritten [-Werror=override-init]
    drivers/mfd/ab8500-debugfs.c:470:11: note: (near initialization for 'ab8500_debug_ranges[18]')
    
    In both cases, one initialization looks valid, the other one overrides
    it with an empty one. I'm removing the empty overrides now, assuming
    they were not intentional.
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/mfd/ab8500-debugfs.c b/drivers/mfd/ab8500-debugfs.c
index 0aecd7bd35f8..c23104b277c4 100644
--- a/drivers/mfd/ab8500-debugfs.c
+++ b/drivers/mfd/ab8500-debugfs.c
@@ -463,14 +463,6 @@ static struct ab8500_prcmu_ranges ab8500_debug_ranges[AB8500_NUM_BANKS] = {
 			},
 		},
 	},
-	[0x11] = {
-		.num_ranges = 0,
-		.range = NULL,
-	},
-	[0x12] = {
-		.num_ranges = 0,
-		.range = NULL,
-	},
 	[0x13] = {
 		.num_ranges = 0,
 		.range = NULL,

^ permalink raw reply related

* [PATCH 6/7] mfd: omap-usb-host: Return value is not 'const int'
From: Arnd Bergmann @ 2016-09-15 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915104521.14286-7-lee.jones@linaro.org>

On Thursday, September 15, 2016 11:45:20 AM CEST Lee Jones wrote:
> Change from 'const int' to just 'int'.
> 
> Cc: <robh+dt@kernel.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 

Looks good, I seem to have done the same thing. Also didn't submit,
so keep your version.

I tend to include the exact warning in the changelog, which here was

drivers/mfd/omap-usb-host.c:165:18: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]

	Arnd

^ permalink raw reply

* [PATCH 0/7] mfd: Fix all W=1 warnings
From: Arnd Bergmann @ 2016-09-15 11:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915104521.14286-1-lee.jones@linaro.org>

On Thursday, September 15, 2016 11:45:14 AM CEST Lee Jones wrote:
> It really is as simple as it sounds!
> 

I seem to have one that you didn't find.

	Arnd

8<--------
>From 6c63171eba728fc6d8bc6358884deaccd4cee33d Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 4 Aug 2016 15:28:51 +0200
Subject: [PATCH] mfd: tps65912: move regmap config into core driver

When building with extra warnings enabled, most files including
linux/mfd/tps65912.h warn about a static variable defined in the
header:

include/linux/mfd/tps65912.h:331:35: warning: 'tps65912_regmap_config' defined but not used [-Wunused-const-variable=]

We also duplicate the data structure between the i2c and spi front-end
drivers. Moving it into the driver code avoids the warning and
the duplication.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/mfd/tps65912-core.c b/drivers/mfd/tps65912-core.c
index a88cfa80dbc4..f33567bc428d 100644
--- a/drivers/mfd/tps65912-core.c
+++ b/drivers/mfd/tps65912-core.c
@@ -77,6 +77,23 @@ static struct regmap_irq_chip tps65912_irq_chip = {
 	.init_ack_masked = true,
 };
 
+static const struct regmap_range tps65912_yes_ranges[] = {
+	regmap_reg_range(TPS65912_INT_STS, TPS65912_GPIO5),
+};
+
+static const struct regmap_access_table tps65912_volatile_table = {
+	.yes_ranges = tps65912_yes_ranges,
+	.n_yes_ranges = ARRAY_SIZE(tps65912_yes_ranges),
+};
+
+const struct regmap_config tps65912_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_table = &tps65912_volatile_table,
+};
+EXPORT_SYMBOL_GPL(tps65912_regmap_config);
+
 int tps65912_device_init(struct tps65912 *tps)
 {
 	int ret;
diff --git a/include/linux/mfd/tps65912.h b/include/linux/mfd/tps65912.h
index 1a603701550e..b25d0297ba88 100644
--- a/include/linux/mfd/tps65912.h
+++ b/include/linux/mfd/tps65912.h
@@ -319,21 +319,7 @@ struct tps65912 {
 	struct regmap_irq_chip_data *irq_data;
 };
 
-static const struct regmap_range tps65912_yes_ranges[] = {
-	regmap_reg_range(TPS65912_INT_STS, TPS65912_GPIO5),
-};
-
-static const struct regmap_access_table tps65912_volatile_table = {
-	.yes_ranges = tps65912_yes_ranges,
-	.n_yes_ranges = ARRAY_SIZE(tps65912_yes_ranges),
-};
-
-static const struct regmap_config tps65912_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-	.cache_type = REGCACHE_RBTREE,
-	.volatile_table = &tps65912_volatile_table,
-};
+extern const struct regmap_config tps65912_regmap_config;
 
 int tps65912_device_init(struct tps65912 *tps);
 int tps65912_device_exit(struct tps65912 *tps);

^ permalink raw reply related

* [PATCH v3 00/14] Clock improvement for video playback
From: Patrice Chotard @ 2016-09-15 11:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <af43c05e-4485-d756-3362-19a76aa40307@st.com>



On 09/15/2016 08:57 AM, Patrice Chotard wrote:
> 
> 
> On 09/14/2016 08:36 PM, Stephen Boyd wrote:
>> On 08/29, gabriel.fernandez at st.com wrote:
>>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>>
>>> v3:
>>>   - Rebase to v4.8-rc1
>>>   - Tipo fix in st,clkgen-pll.txt
>>>   - Add Ack of Peter for the series
>>>   - Add missed patch: "ARM: DT: STiH4xx: Simplify clock binding of STiH4xx platforms"
>>>
>>> v2:
>>>   - Simpliflication of clock binding
>>>     remark from Rob https://lkml.org/lkml/2016/5/25/492
>>>   - Suppression of stih415-416 support for the clocks (in order
>>> 	to help simplification of clock binding)
>>>     (others patchs for the machine and drivers will come)
>>>
>>> This serie allows to increase video resolutions and make audio
>>> adjustment during a video playback.
>>>
>>> Gabriel Fernandez (14):
>>>   drivers: clk: st: Remove stih415-416 clock support
>>>   drivers: clk: st: Simplify clock binding of STiH4xx platforms
>>>   ARM: DT: STiH4xx: Simplify clock binding of STiH4xx platforms
>>>   drivers: clk: st: Add fs660c32 synthesizer algorithm
>>>   drivers: clk: st: Add clock propagation for audio clocks
>>>   drivers: clk: st: Handle clk synchronous mode for video clocks
>>>   ARM: DT: STiH407: Enable clock propagation for audio clocks
>>>   ARM: DT: STiH410: Enable clock propagation for audio clocks
>>>   ARM: DT: STiH418: Enable clock propagation for audio clocks
>>>   ARM: DT: STiH407: Enable synchronous clock mode for video clocks
>>>   ARM: DT: STiH410: Enable synchronous clock mode for video clocks
>>>   ARM: DT: STiH418: Enable synchronous clock mode for video clocks
>>>   ARM: DT: STi: STiH407: clock configuration to address 720p and 1080p
>>>   ARM: DT: STi: STiH410: clock configuration to address 720p and 1080p
>>
> 
> Hi Stephen
> 
>> The order of patches intermingles clk changes and dts changes.
>> I'd prefer to not take any patches for dts through the clk tree,
>> so can those be deferred to an arm-soc PR? 
> 
> No problem, i will take all DTS patches and submit a pull request to arm-soc.
> 
> Assuming that works,
>> I'd just pick 1-2, and 4-6 into the clk tree and the rest can go
>> on top through arm-soc.

Stephen

Just to confirm, these patches will be merged into clk-next tree ? so dedicated for v4.9 ?

Thanks

Patrice

>>
> 
> Perfect 
> 
> Thanks
> 
> 
> 

^ permalink raw reply

* [GIT PULL] Greybus driver subsystem for 4.9-rc1
From: Bryan O'Donoghue @ 2016-09-15 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915112029.GC6718@leverpostej>

On Thu, 2016-09-15 at 12:20 +0100, Mark Rutland wrote:
> Hi,
> 
> More questions below. Perhaps some of these will be implicitly
> answered
> when the linearised patches appear, and I'm happy to wait until then
> to
> continue the discussion, as I suspect otherwise we're all likely to
> end
> up exasperated.
> 
> Please do Cc me on those.
> 
> Regardless, until those appear and a reasonable time has been given
> for
> replies, my comments regarding the lack of review stand, as does my
> NAK
> for the series.
> 
> On Thu, Sep 15, 2016 at 11:35:56AM +0100, Bryan O'Donoghue wrote:
> > 
> > On Thu, 2016-09-15 at 11:13 +0100, Mark Rutland wrote:
> > > 
> > > On Thu, Sep 15, 2016 at 10:35:33AM +0100, Bryan O'Donoghue wrote:
> > > > 
> > > > ?
> > > I don't think the history matters,?
> > Your comment seemed to indicate you thought we were reading a
> > architectural timer directly - which we aren't.
> Sure, and as I pointed out, the comment in the HEAD commit still
> claims
> it does, even if the code doesn't. This is at best, confusing, and
> the
> history of how it came to be there doesn't really matter...

TBH a whole git history will invariably contain things developers did,
thought better of and then backed out as is the case here.

> 
> > 
> > > 
> > > and I don't think that one can rely
> > > on get_cycles() in this manner outside of arch code.
> > I don't follow your meaning. What's wrong with get_cycles() ?
> > You've
> > already said you don't think reading an architectural timer
> > directly is
> > correct.
> I pointed out a number of issues in my previous reply.

On MSM8994 the timer backing get_cycles() is one of the MMIO
architectural timers (which is why I switched over in the end). There's
not much else that can be done bar custom silicon - this particular
timer is as good as it gets, more of a "how do we synchronise time with
the hardware we have" than a "lets design in a feature to synchronise
time" - which was something we were focusing in on for later
silicon...?

> 
> For example, you have absolutely no guarantee as to what backs
> get_cycles(). Despite this, the code assumes that get_cycles() is
> backed
> by something running at the frequency described in a
> "google,greybus-frame-time-counter" node.
> 
> Even if this *happens* to match what some piece of arch code provides
> today on some platform, it is in no way *guaranteed*.

That's the point though, if you declare "google,greybus-frame-time-
counter" in your platform code - then you can use 'get_cycles()' in
this manner - if not - then you need to take steps in your own new
platform to provide that same level of functionality. You could switch
to an MSM8996 or an MSM8998 declare this node and bob's your uncle.

OTOH declaring this node on x86 would be a bit pointless. You'd be
better off providing a timer on a PCI bar, and binding that into
greybus with some x86/x86-platform code...

> 
> > 
> > The objective is to read one of the free-running counters in
> > MSM8994,
> > clocked by the PMIC. The refclk provided by PMIC is distributed to
> > each
> > processor in the system.
> > 
> > > 
> > > ?Looking at the
> > > state of the tree [1] as of the final commit [2] in the greybus
> > > branch,
> > > my points still stand:
> > > 
> > > * The "google,greybus-frame-time-counter" node is superfluous. It
> > > does
> > > ? not describe a particular device,
> > It describes a timer running @ 19.2MHz, clocked by PMIC refclk.
> ... which you assume is whatever backs get_cycles(), which you in
> practice assume is the architected timer. For which we *already* have
> a
> binding and driver.

It's a requirement rather than assumption. If you declare that node,
it's assumed the timer driving get_cycles() does what it says on the
greybus-frame-time-counter tin.

> > > ?and duplicates information we have ? elsewhere.
> > Can you give an example ?
> Trivially, the CNTFRQ register in the architected timer (which is
> common
> across MMIO/sysreg), which you can query with arch_timer_get_rate().
> 
> Note that isn't guaranteed to match get_cycles() either. You need a
> better API to call.

In that case a DT entry makes sense I'd say.

> You should definitely interrogate the relevant driver, somehow.

Hrmm. TBH if we are ruling out arch_timer_get_rate() then I think a DT
entry (which BTW is greybus specific) is the more intelligent way
forward. The greybus platform implementer needs to understand the
dependencies and take action to meet those dependencies should he or
she wish to support this feature.

I'm not opposed necessarily to calling arch_timer_get_rate() instead of
a DT binding, assuming it works, and drawing a line under it for
MSM8994. As I've said it's up to a system architect for other platforms
to go and do the necessary design to support this feature and this will
almost certainly require new platform code both here and in other
places anyway.

> 
> Without a higher-level view of what you're trying to achieve, it's
> not
> clear to me whether get_cycles() is the right interface.

I appreciate that.

---
bod

^ permalink raw reply

* [PATCH 00/19] [RESEND] Remove STiH415 and STiH416 SoC platform support
From: Arnd Bergmann @ 2016-09-15 12:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160915070139.GA14319@griffinp-ThinkPad-X1-Carbon-2nd>

On Thursday, September 15, 2016 8:01:39 AM CEST Peter Griffin wrote:
> 
> STiH415 I'm sure never shipped. I'm reasonably sure STiH416 didn't
> either. These SoCs were considered legacy even when I was at ST
> ~3 years ago.
> 
> Also remember these are STB SoC's, so JTAG fuses are blown in
> production boxes, and also full security is enabled. This means the
> primary bootloader will only boot a signed kernel. So if a end user
> did happen to have a box they would be unable to upgrade their kernel.
> 
> From the landing team perspective they were interesting in that they
> shared many IPs with the STiH407 family on which future chipsets were
> based, and were available to us when that silicon was harder to get
> hold of. So we used it as a vehicle for upstreaming so that upstream
> support was already quite good when STiH407 silicon did land on our
> desk.

Ok, makes sense. I did stumble over one machine basedon STiH412
the other day [1], but there probably isn't much shared with that
one. Since this a NAS server rather than an STB box, it's probably
less locked-down and potentially a target for OpenWRT or similar.

	Arnd

[1] http://www.heise.de/preisvergleich/synology-diskstation-ds216play-16tb-a1400885.html

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-09-15 12:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8360154.kzsE8V8pTW@wuerfel>

Hi Arnd

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 15 September 2016 09:22
> To: Gabriele Paoloni
> Cc: linux-arm-kernel at lists.infradead.org; Yuanzhichang;
> devicetree at vger.kernel.org; lorenzo.pieralisi at arm.com; minyard at acm.org;
> gregkh at linuxfoundation.org; benh at kernel.crashing.org; John Garry;
> will.deacon at arm.com; linux-kernel at vger.kernel.org; xuwei (O); Linuxarm;
> linux-serial at vger.kernel.org; linux-pci at vger.kernel.org;
> zourongrong at gmail.com; liviu.dudau at arm.com; kantyzc at 163.com;
> zhichang.yuan02 at gmail.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
> >
> > From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> >
> > I quote:
> > "There shall be an entry in the "ranges" property for each
> > of the Memory and/or I/O spaces if that address space is
> > mapped through the bridge."
> >
> > It seems to me that it is ok to have 1:1 address mapping and that
> > therefore of_translate_address() should fail if "ranges" is not
> > present.
> 
> The key here is the definition of "mapped through the bridge".
> I can only understand this as "directly mapped", i.e. an I/O
> port of the child bus corresponds directly to a memory address
> on the parent bus, but this is not the case here.
> 
> The problem with adding the mapping here is that it looks
> like it should be valid to create a page table entry for
> the address returned from the translation and access it through
> a pointer dereference, but that is clearly not possible.

I understand that somehow we are abusing of the ranges property
here however the point is that with the current implementation ranges
is needed because otherwise the ipmi driver probe will fail here:

of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
-> of_translate_address -> __of_translate_address

Now we had a bit of discussion internally and to avoid
having ranges we came up with two possible solutions:

1) Using bit 3 of phys.hi cell in 2.2.1 of
http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
This would mean reworking of_bus_isa_get_flags in 
http://lxr.free-electrons.com/source/drivers/of/address.c#L398
and setting a new flag to be checked in __of_address_to_resource

2) Adding a property in the bindings of each device that is
a child of our LPC bus and modify __of_address_to_resource
to check if the property is in the DT and eventually bypass
of_translate_address

However in both 1) and 2) there are some issues:
in 1) we are not complying with the isa binding doc (we use
a bit that should be zero); in 2) we need to modify the
bindings documentation of the devices that are connected
to our LPC controller (therefore modifying other devices
bindings to fit our special case).

I think that maybe having the 1:1 range mapping doesn't
reflect well the reality but it is the less painful
solution...

What's your view?
 
Many Thanks

Gab

> 
> > This is also explained quite well in
> > http://lxr.free-electrons.com/source/drivers/of/address.c#L490
> >
> > what do you think?
> 
> This is a separate issue, and only relevant for Apple Macintosh
> machines as well as the PA-Semi sdc.
> 
> 	Arnd

^ permalink raw reply

* [PATCH 4/5] ARM: dts: exynos: add support for ISP power domain to exynos4x12 clocks device
From: Marek Szyprowski @ 2016-09-15 12:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160912222852.GH7243@codeaurora.org>

Hi Stephen,

On 2016-09-13 00:28, Stephen Boyd wrote:
> On 09/12, Marek Szyprowski wrote:
>> On 2016-09-08 02:22, Stephen Boyd wrote:
>>> On 09/01, Marek Szyprowski wrote:
>>>> Exynos4412 clock controller contains some additional clocks for FIMC-ISP
>>>> (Camera ISP) subsystem. Registers for those clocks are partially located
>>>> in the SOC area, which belongs to ISP power domain.
>>>>
>>>> This patch extends clock controller node with ISP clock sub-node and link
>>>> (phandle) to ISP power domain.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>   arch/arm/boot/dts/exynos4x12.dtsi | 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
>>>> index 3394bdcf10ae..4daea67546b9 100644
>>>> --- a/arch/arm/boot/dts/exynos4x12.dtsi
>>>> +++ b/arch/arm/boot/dts/exynos4x12.dtsi
>>>> @@ -74,6 +74,11 @@
>>>>   		compatible = "samsung,exynos4412-clock";
>>>>   		reg = <0x10030000 0x20000>;
>>>>   		#clock-cells = <1>;
>>>> +
>>>> +		isp-clock-controller {
>>>> +			compatible = "samsung,exynos4412-isp-clock";
>>>> +			power-domains = <&pd_isp>;
>>>> +		};
>>> Why can't we extend support in power domains code to have
>>> multiple domains for a single device node? i.e. power-domains =
>>> <&pd_isp>, <&pd_foo>, <&pd_bar>, and then pick the right one with
>>> power-domain-names or something like that? Making a subnode
>>> (which seems to turn into a child platform device?) seems like a
>>> quick solution for larger problems.
>> The larger problem here is the fact that clock controller is
>> partially located
>> in different power areas of SoC. Majority of the clock controllers
>> is located
>> in the area which is typically always powered (besides system sleep case),
>> while a few Camera ISP registers are located in the ISP block, which have
>> separate power domain. Having a separate nodes for sub-parts of the
>> device is
>> rather common approach, already practices by some more complex devices.
>>
>> I see some serious design problems with multiple entries in power domains
>> property. First how to show that some part of the device IS NOT in
>> any domain?
> Is that even possible? Every device should be in some power
> domain, even if it's just an "always on" power domain that we
> don't really control from software.

Right now none dts of which I'm aware of doesn't define the power domain for
the parts of the SoC, which are always on and doesn't need any additional
management.

>
>> The question is how the automated assignment to domains would be handled for
>> such case?
> I don't get this part. Do you mean how we indicate to the driver
> which power domain to use at the right time?

This was about Linux device core, which assigns device to its power domain
and ensures that the power domain is in right state during device probe and
then during device operation. Currently core supports only one domain per
device, so the question was which domain to chose if there are more than
one listed.

>
>> The second is related to Linux kernel internals. Right now device
>> drivers are
>> not aware of the power domains - there are no direct calls to power domains
>> code, everything is hidden behind runtime pm which does all the hard work.
> Right. Runtime PM will need to be improved to allow this case.
>
>> Similar situation is on Exynos 542x/5800, which will look more or less like
>> this:
>>
>>          clock: clock-controller at 10010000 {
>>               compatible = "samsung,exynos5420-clock";
>>               reg = <0x10010000 0x30000>;
>>               #clock-cells = <1>;
>> +
>> +            gsc-clock-controller {
>> +                 compatible = "samsung,exynos5420-gsc-clock";
>> +                 power-domains = <&gsc_pd>;
>> +            };
>> +
>> +            isp-clock-controller {
>> +                 compatible = "samsung,exynos5420-isp-clock";
>> +                 power-domains = <&isp_pd>;
>> +            };
>> +
>> +            mfc-clock-controller {
>> +                 compatible = "samsung,exynos5420-mfc-clock";
>> +                 power-domains = <&mfc_pd>;
>> +            };
>> +
>> +            msc-clock-controller {
>> +                 compatible = "samsung,exynos5420-msc-clock";
>> +                 power-domains = <&msc_pd>;
>> +            };
>> +
>> +            disp-clock-controller {
>> +                 compatible = "samsung,exynos5420-disp-clock";
>> +                 power-domains = <&disp_pd>;
>> +            };
>>           };
>>
>> The patch is not yet ready, so I didn't include it in this patchset.
> Ok. From a DT perspective the sub-nodes seem to be a workaround
> for how the linux device model is mapped to power domains. I'm
> not sure we want to make subnodes in the clk controller just to
> make sub devices that we can target from the clk registration
> path. Those sub nodes aren't devices at all. I understand why
> it's being done this way, I just don't see how it fits into DT
> design methodologies.

I don't think that using sub-nodes for describing details of the given
hardware block is something uncommon in DT. Please check pin control
or PMICs (especially regulator providers). Same for various "port"
sub-nodes often used by various video/display devices and bridges.

The above presented method describes well which sub-part of clock
controller is placed in which power domain.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply

* [RESEND PATCH v2 1/4] pinctrl: amlogic: gxbb: add spi nor pins
From: Linus Walleij @ 2016-09-15 12:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473779542-2635-1-git-send-email-jbrunet@baylibre.com>

On Tue, Sep 13, 2016 at 5:12 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:

> Add EE domains pins for the SPI flash controller
>
> Acked-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] pinctrl: stm32: add IRQ_DOMAIN_HIERARCHY dependency
From: Linus Walleij @ 2016-09-15 12:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160914101323.3254566-1-arnd@arndb.de>

On Wed, Sep 14, 2016 at 12:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> The newly added irqchip support for the stm32 pinctrl driver uses
> hierarchical IRQ domains as provided by the NVIC primary irqchip.
> This works great for any configuration that may be relevant on
> stm32, but when doing compile-testing (randconfig), we can
> enable it without NVIC or any other primary irqchip that
> enables IRQ_DOMAIN_HIERARCHY:
>
> drivers/pinctrl/stm32/pinctrl-stm32.c:212:13: error: 'irq_chip_eoi_parent' undeclared here (not in a function)
> drivers/pinctrl/stm32/pinctrl-stm32.c:213:20: error: 'irq_chip_mask_parent' undeclared here (not in a function)
> drivers/pinctrl/stm32/pinctrl-stm32.c:214:20: error: 'irq_chip_unmask_parent' undeclared here (not in a function)
> drivers/pinctrl/stm32/pinctrl-stm32.c:215:20: error: 'irq_chip_set_type_parent' undeclared here (not in a function)
>
> This adds a Kconfig dependency to limit compile-testing to
> configurations that have IRQ_DOMAIN_HIERARCHY already enabled.
> It's not obvious whether we should use 'depends on' or 'select'
> here, I think either one works, with 'depends on' being more
> intuitive, while 'select' would be less likely to cause dependency
> loops.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 0eb9f683336d ("pinctrl: Add IRQ support to STM32 gpios")

Ooops patch applied, thanks a lot!

Yours,
Linus Walleij

^ permalink raw reply

* [RESEND PATCH v2 2/4] pinctrl: amlogic: gxbb: add nand pins
From: Linus Walleij @ 2016-09-15 12:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473779542-2635-2-git-send-email-jbrunet@baylibre.com>

On Tue, Sep 13, 2016 at 5:12 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:

> Add EE domains pins for the NAND flash controller.
> Even tough we have no driver for the NAND flash controller yet, we need
> to have these pins in pinctrl as the actual pin are shared with the spifc
> controller. The bootloader on the S905-P200 setup pinmux for the NAND
> controller so we need the kernel to properly deactivate this if necessary.
>
> Acked-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* v4.8-rc: GSM audio causes trouble
From: Pavel Machek @ 2016-09-15 12:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi!

I was trying to improve GSM call quality, and hit problems in
v4.8-rc. Sound only worked for a while, then I tried to kill
cmtspeech_ofono_test, and could not, not even with -9 and could not
even reboot.

I went back to v4.1 (ok, quite far, I see), and problems are gone.

Does it work for you? Any ideas what to try... besides bisect?

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH] pintctrl: amlogic: gxbb: add i2c pins
From: Linus Walleij @ 2016-09-15 12:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473846328-17339-1-git-send-email-jbrunet@baylibre.com>

On Wed, Sep 14, 2016 at 11:45 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:

> Add EE domains pins for the i2c devices A,B,C
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Patch applied with Kevin's ACK.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 1/3] misc: Add Aspeed BT IPMI host driver
From: Corey Minyard @ 2016-09-15 12:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3e2658cc-7089-abd2-23b3-ffd468edbd11@kaod.org>

On 09/15/2016 01:51 AM, C?dric Le Goater wrote:
> On 09/12/2016 10:33 PM, Corey Minyard wrote:
>> On 09/12/2016 02:15 PM, Arnd Bergmann wrote:
>>> On Monday, September 12, 2016 1:55:40 PM CEST Corey Minyard wrote:
>>>> On 09/02/2016 08:22 AM, C?dric Le Goater wrote:
>>>>> Hello,
>>>>>
>>>>> Adding Corey in cc: . I guess I should have done that in the first place.
>>>> Yes, probably so.  I've been travelling and didn't see it on the mailing
>>>> lists until now.
>>>>
>>>> There is already a BT driver in the kernel, in drivers/char/ipmi, why
>>>> won't that work?
>>> The new driver is the host side (running on the BMC), the existing one
>>> is the client (running on the PC).
>>>
>>>      Arnd
>> Ok, that's not really clear from the documentation or the Kconfig.
>> In the IPMI spec the "host" side is the computer side, not the BMC
>> side.  Like:
>>
>>     11.6.1 BT Host Interface Registers
>>     The Host BT interface provides an independent set of registers and
>>     interrupts to allow the Host driver to
>>     communicate with the baseboard management controller without
>>     conflicting with the O/S ACPI driver.
>>
>> In light of that, this should probably be named the bt-bmc driver.
>>
>> I haven't reviewed this in detail, but I'm ok with putting it in
>> drivers/char/ipmi.  The state machine part looks reasonably
>> generic.  The configuration part isn't, but that could be split
>> out later if necessary.
> what do you mean by configuration ? I am ready to send a v2. May be
> I can add a few other things.

The part that sets everything up, basically everything from
bt_host_fops down.

If you were to use this on another system, that code would have to be
made more generic and the machine-specific part split into different
files, but that's not necessary now, I don't think.

-corey

> Thanks,
>
> C.

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-09-15 12:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F871F72@lhreml507-mbx>

On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni wrote:
> > -----Original Message-----
> > On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
> > >
> > > From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
> > > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > >
> > > I quote:
> > > "There shall be an entry in the "ranges" property for each
> > > of the Memory and/or I/O spaces if that address space is
> > > mapped through the bridge."
> > >
> > > It seems to me that it is ok to have 1:1 address mapping and that
> > > therefore of_translate_address() should fail if "ranges" is not
> > > present.
> > 
> > The key here is the definition of "mapped through the bridge".
> > I can only understand this as "directly mapped", i.e. an I/O
> > port of the child bus corresponds directly to a memory address
> > on the parent bus, but this is not the case here.
> > 
> > The problem with adding the mapping here is that it looks
> > like it should be valid to create a page table entry for
> > the address returned from the translation and access it through
> > a pointer dereference, but that is clearly not possible.
> 
> I understand that somehow we are abusing of the ranges property
> here however the point is that with the current implementation ranges
> is needed because otherwise the ipmi driver probe will fail here:
> 
> of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
> -> of_translate_address -> __of_translate_address
> 
> Now we had a bit of discussion internally and to avoid
> having ranges we came up with two possible solutions:
> 
> 1) Using bit 3 of phys.hi cell in 2.2.1 of
> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> This would mean reworking of_bus_isa_get_flags in 
> http://lxr.free-electrons.com/source/drivers/of/address.c#L398
> and setting a new flag to be checked in __of_address_to_resource
> 
> 2) Adding a property in the bindings of each device that is
> a child of our LPC bus and modify __of_address_to_resource
> to check if the property is in the DT and eventually bypass
> of_translate_address
> 
> However in both 1) and 2) there are some issues:
> in 1) we are not complying with the isa binding doc (we use
> a bit that should be zero); in 2) we need to modify the
> bindings documentation of the devices that are connected
> to our LPC controller (therefore modifying other devices
> bindings to fit our special case).
> 
> I think that maybe having the 1:1 range mapping doesn't
> reflect well the reality but it is the less painful
> solution...
> 
> What's your view?

We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
and that should be enough to translate the I/O port number.

The only part we need to change here is to not go through
the crazy conversion all the way from PCI I/O space to a
physical address and back to a (logical) port number
that we do today with of_translate_address/pci_address_to_pio.

I can think of a several of ways to fix __of_address_to_resource
to just do the right thing according to the ISA binding to
make the normal drivers work.

The easiest solution is probably to hook into the
"taddr == OF_BAD_ADDR" case in __of_address_to_resource
and add a lookup for ISA buses there, and instead check
if some special I/O port operations were registered
for the port number, using an architecture specific
function that arm64 implements. Other architectures
like x86 that don't have a direct mapping between I/O
ports and MMIO addresses would implement that same
function differently.

	Arnd

^ permalink raw reply

* [REGRESSION] BISECTED : ethernet gadget not working anymore on several atmel platforms
From: Richard Genoud @ 2016-09-15 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

Since:
commit c32b5bcfa3c43a3c9bb59f65b5e76adb7384c4c8
Author: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Date:   Tue Jul 12 22:45:59 2016 +0200

    ARM: dts: at91: Fix USB endpoint nodes

    Endpoint nodes have a reg property. Add their mandatory unit-address.

Booting with an USB gadget configured as Ethernet gadget + RNDIS
Gives a warning:

On AT91SAM9x5 platform (g35) :
g_ether gadget: high-speed config #1: CDC Ethernet (ECM)
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/usb/gadget/udc/core.c:264
usb_ep_queue+0x4c/0xec
CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-rc6-00071-gcc183ac148d0 #134
Hardware name: Atmel AT91SAM9
[<c000f164>] (unwind_backtrace) from [<c000d158>] (show_stack+0x20/0x24)
[<c000d158>] (show_stack) from [<c01ba940>] (dump_stack+0x20/0x28)
[<c01ba940>] (dump_stack) from [<c0017f30>] (__warn+0xe0/0x10c)
[<c0017f30>] (__warn) from [<c001802c>] (warn_slowpath_null+0x30/0x38)
[<c001802c>] (warn_slowpath_null) from [<c02cd78c>] (usb_ep_queue+0x4c/0xec)
[<c02cd78c>] (usb_ep_queue) from [<c02d55b8>] (ecm_do_notify+0x134/0x150)
[<c02d55b8>] (ecm_do_notify) from [<c02d5840>] (ecm_set_alt+0x14c/0x154)
[<c02d5840>] (ecm_set_alt) from [<c02c9144>] (composite_setup+0xa2c/0x13c0)
[<c02c9144>] (composite_setup) from [<c02d3a90>] (usba_udc_irq+0x888/0xb4c)
[<c02d3a90>] (usba_udc_irq) from [<c00458a0>]
(__handle_irq_event_percpu+0x74/0x1dc)
[<c00458a0>] (__handle_irq_event_percpu) from [<c0045a34>]
(handle_irq_event_percpu+0x2c/0x68)
[<c0045a34>] (handle_irq_event_percpu) from [<c0045aa8>]
(handle_irq_event+0x38/0x4c)
[<c0045aa8>] (handle_irq_event) from [<c0048a94>]
(handle_fasteoi_irq+0xa0/0x114)
[<c0048a94>] (handle_fasteoi_irq) from [<c004512c>]
(generic_handle_irq+0x28/0x38)
[<c004512c>] (generic_handle_irq) from [<c00451cc>]
(__handle_domain_irq+0x90/0xb8)
[<c00451cc>] (__handle_domain_irq) from [<c0009450>] (aic_handle+0xb0/0xb8)
[<c0009450>] (aic_handle) from [<c000dca8>] (__irq_svc+0x68/0x84)
Exception stack(0xc788dc68 to 0xc788dcb0)
dc60:                   00000151 000004f6 00000151 c7075b70 c89693b0 c8964000
dc80: c79c7400 000060e8 c79aa000 c7bd55c0 00000023 c788dd3c c788dd00 c788dcb8
dca0: c0319cc0 c0319cd4 80000013 ffffffff
[<c000dca8>] (__irq_svc) from [<c0319cd4>] (ubi_attach_fastmap+0x858/0xcc8)
[<c0319cd4>] (ubi_attach_fastmap) from [<c0278744>]
(ubi_scan_fastmap+0x644/0x854)
[<c0278744>] (ubi_scan_fastmap) from [<c027542c>] (scan_fast+0x10c/0x17c)
[<c027542c>] (scan_fast) from [<c02757c0>] (ubi_attach+0xb0/0x21c)
[<c02757c0>] (ubi_attach) from [<c0269520>] (ubi_attach_mtd_dev+0x320/0x820)
[<c0269520>] (ubi_attach_mtd_dev) from [<c06b42d4>] (ubi_init+0x1a8/0x248)
[<c06b42d4>] (ubi_init) from [<c0697df0>] (do_one_initcall+0xb4/0x16c)
[<c0697df0>] (do_one_initcall) from [<c0697fb0>]
(kernel_init_freeable+0x108/0x1d4)
[<c0697fb0>] (kernel_init_freeable) from [<c04e3c84>] (kernel_init+0x18/0x100)
[<c04e3c84>] (kernel_init) from [<c000a470>] (ret_from_fork+0x14/0x24)
---[ end trace 8150ab0dc38dec93 ]---

Same on SAMA5D3:
g_ether gadget: high-speed config #1: CDC Ethernet (ECM)
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at drivers/usb/gadget/udc/core.c:264
usb_ep_queue+0x5c/0x64
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 4.8.0-rc6+ #135
Hardware name: Atmel SAMA5
[<c010ce70>] (unwind_backtrace) from [<c010a88c>] (show_stack+0x10/0x14)
[<c010a88c>] (show_stack) from [<c0115bb0>] (__warn+0xe4/0xfc)
[<c0115bb0>] (__warn) from [<c0115c78>] (warn_slowpath_null+0x20/0x28)
[<c0115c78>] (warn_slowpath_null) from [<c03cc608>] (usb_ep_queue+0x5c/0x64)
[<c03cc608>] (usb_ep_queue) from [<c03d1124>] (ecm_do_notify+0xec/0x14c)
[<c03d1124>] (ecm_do_notify) from [<c03d130c>] (ecm_set_alt+0x74/0x150)
[<c03d130c>] (ecm_set_alt) from [<c03c9364>] (composite_setup+0xb84/0x1570)
[<c03c9364>] (composite_setup) from [<c03cf364>] (usba_udc_irq+0x878/0xd4c)
[<c03cf364>] (usba_udc_irq) from [<c013f12c>]
(__handle_irq_event_percpu+0x88/0x124)
[<c013f12c>] (__handle_irq_event_percpu) from [<c013f1e4>]
(handle_irq_event_percpu+0x1c/0x58)
[<c013f1e4>] (handle_irq_event_percpu) from [<c013f248>]
(handle_irq_event+0x28/0x3c)
[<c013f248>] (handle_irq_event) from [<c0141c8c>]
(handle_fasteoi_irq+0xa0/0x168)
[<c0141c8c>] (handle_fasteoi_irq) from [<c013e87c>]
(generic_handle_irq+0x24/0x34)
[<c013e87c>] (generic_handle_irq) from [<c013ead8>]
(__handle_domain_irq+0x54/0xa8)
[<c013ead8>] (__handle_domain_irq) from [<c010b34c>] (__irq_svc+0x6c/0x90)
[<c010b34c>] (__irq_svc) from [<c0130be4>] (finish_task_switch+0x4c/0x144)
[<c0130be4>] (finish_task_switch) from [<c054b9d0>] (schedule+0x44/0x9c)
[<c054b9d0>] (schedule) from [<c054bbd4>] (schedule_preempt_disabled+0xc/0x10)
[<c054bbd4>] (schedule_preempt_disabled) from [<c01374b8>]
(cpu_startup_entry+0x98/0xf4)
[<c01374b8>] (cpu_startup_entry) from [<c0800c38>] (start_kernel+0x35c/0x368)
[<c0800c38>] (start_kernel) from [<20008078>] (0x20008078)
---[ end trace 27b3e952067e1be6 ]---

And I guess it's the same on g45, 9rl, sama5d[2-4].

Bisect done with a 4.8-rc6 kernel.
Here's the bisect log:
git bisect start 'arch/arm/boot/dts/'
# good: [882bd9fc46321c9d4721b376039a142cbfe8a17a] usb: gadget: udc:
atmel: Also get regmap for at91sam9x5-pmc
git bisect good 882bd9fc46321c9d4721b376039a142cbfe8a17a
# bad: [694d0d0bb2030d2e36df73e2d23d5770511dbc8d] Linux 4.8-rc2
git bisect bad 694d0d0bb2030d2e36df73e2d23d5770511dbc8d
# good: [ab4b4340c7d74310a132cb457665bf3d98fdff79] Merge tag
'imx-dt-4.8' of
git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux into
next/dt
git bisect good ab4b4340c7d74310a132cb457665bf3d98fdff79
# good: [349f9d5239c3dae3b984679fc9ee00b8deec542b] Merge tag
'sunxi-dt-for-4.8' of
https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux into
next/dt
git bisect good 349f9d5239c3dae3b984679fc9ee00b8deec542b
# bad: [262a5d84b2aea8deb67eca813dc3008836330596] Merge tag
'sunxi-dt-for-4.8-2-bis' of
https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux into
next/dt
git bisect bad 262a5d84b2aea8deb67eca813dc3008836330596
# good: [cda1c2bdf6df90c12363877fda594a0a1b7ac74a] Merge tag
'sti-late-v4.8' of
git://git.kernel.org/pub/scm/linux/kernel/git/pchotard/sti into
next/late
git bisect good cda1c2bdf6df90c12363877fda594a0a1b7ac74a
# good: [a0e4eb4b81b44663f21b289d8126ca6f6dc8a863] ARM: dts: sun8i:
Use sun8i-reference-design-tablet for polaroid mid2809pxe04
git bisect good a0e4eb4b81b44663f21b289d8126ca6f6dc8a863
# bad: [d3c1c7181f55f3464529decad189f2317a2932da] ARM: dts: at91:
vinco: fix regulator name
git bisect bad d3c1c7181f55f3464529decad189f2317a2932da
# bad: [a63f6a64cc1e78366382657cab70772240b6dcc2] ARM: dts: at91:
sama5d3_xplained: fix regulator name
git bisect bad a63f6a64cc1e78366382657cab70772240b6dcc2
# bad: [c32b5bcfa3c43a3c9bb59f65b5e76adb7384c4c8] ARM: dts: at91: Fix
USB endpoint nodes
git bisect bad c32b5bcfa3c43a3c9bb59f65b5e76adb7384c4c8
# good: [c94afa132b6d7f8e412a12da44da6203d2cd23d4] ARM: dts: at91: Fix
ADC trigger nodes
git bisect good c94afa132b6d7f8e412a12da44da6203d2cd23d4
# first bad commit: [c32b5bcfa3c43a3c9bb59f65b5e76adb7384c4c8] ARM:
dts: at91: Fix USB endpoint nodes

on xplained board, I used at91-sama5d3_xplained.dtb and sama5_defconfig plus :
CONFIG_USB_ETH=y
CONFIG_USB_ETH_RNDIS=y

^ permalink raw reply

* [RESEND PATCH] arm64: kgdb: fix single stepping
From: Jason Wessel @ 2016-09-15 12:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5fd52bac-a303-81c8-1b91-daa55d6cd53f@linaro.org>

On 09/15/2016 05:41 AM, Daniel Thompson wrote:
> On 15/09/16 08:56, AKASHI Takahiro wrote:
>> On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:
>>> Hi Akashi,
>>>
>>> On Tue, Apr 21, 2015 at 02:13:13AM +0100, AKASHI Takahiro wrote:
>>>> Could you please review my patch below?
>>>> See also arm64 maintainer's comment:
>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html
>>>
>>> -ETIMEDOUT waiting for the kdgb folk to comment. Ppeople have reported
>>> that this patch is required for kgdb to work correctly on arm64, so I'm
>>> happy to merge it.
>>
>> I'm happy, too.
>
> I'll keep an eye out and FWIW see if I can throw in a review. I'm not
> really one of "kgdb folk" but did examine it fairly deeply in the early
> stages of the FIQ/NMI work (and which has since stopped focussing on kgdb).
>
> I have some equally elderly, albeit rather less critical, kdb patches
> that I should have pushed harder for so I'm sympathetic here ;-)


Hey, if you do happen to have something, why not send it along.  I just asked the linux-next folks to re-activate the kgdb-next branch so that merges can start taking place again.  It appears that the final merge request never even went through from linux-next as I just rebased it and there was a patch in there from Daniel dating back to over a year ago.

If there is work out there that needs merging and reviews lets get it done.  I had been kind of stuck in time on what seems like the stone age 3.14 kernel projects until recently, but jumped back into mainline development about a month ago.

I have no objection to merging the ARM64 single step patch and have comments in a separate mail that follows with respect to the patch.


Cheers,
Jason.

^ permalink raw reply

* [PATCH 13/19] pinctrl: st: Remove STiH415/6 SoC pinctrl driver support.
From: Linus Walleij @ 2016-09-15 12:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473859677-9231-14-git-send-email-peter.griffin@linaro.org>

On Wed, Sep 14, 2016 at 3:27 PM, Peter Griffin <peter.griffin@linaro.org> wrote:

> STiH415/6 SoC support is being removed from the kernel.
> This patch updates the ST pinctrl driver and removes
> references to these obsolete platforms. As some structures
> referenced by STiH407 based configuration were shared with
> STiH416 we update these names to match the remaining
> supported platform.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Cc: <linus.walleij@linaro.org>
> Cc: <linux-gpio@vger.kernel.org>

Patch applied.

Yours,
Linus Walleij

^ 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