* [PATCH 4/5] tty: serial: sh-sci: Hide DMA config question
From: Geert Uytterhoeven @ 2017-11-30 13:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman, Magnus Damm, Yoshinori Sato,
Rich Felker
Cc: Jiri Slaby, linux-serial, linux-renesas-soc, uclinux-h8-devel,
linux-sh, Geert Uytterhoeven
In-Reply-To: <1512047522-16312-1-git-send-email-geert+renesas@glider.be>
On most Renesas ARM platforms, the SCIF serial ports can be used with
DMA, so most users will want DMA support to be enabled.
On SuperH platforms, SCI(F) serial ports cannot be used with DMA yet
(see also commit 219fb0c1436e4893 ("serial: sh-sci: Remove the platform
data dma slave rx/tx channel IDs")), so users will want it disabled to
reduce kernel size.
Hence follow the above rationale to configure the default, unless
CONFIG_EXPERT is enabled.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 952a2c6a9da08fdd..4e6dfb0a762b5807 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -781,8 +781,9 @@ config SERIAL_SH_SCI_EARLYCON
default ARCH_RENESAS || H8300
config SERIAL_SH_SCI_DMA
- bool "DMA support"
+ bool "DMA support" if EXPERT
depends on SERIAL_SH_SCI && DMA_ENGINE
+ default ARCH_RENESAS
config SERIAL_PNX8XXX
bool "Enable PNX8XXX SoCs' UART Support"
--
2.7.4
^ permalink raw reply related
* [PATCH 3/5] tty: serial: sh-sci: Hide earlycon config question
From: Geert Uytterhoeven @ 2017-11-30 13:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman, Magnus Damm, Yoshinori Sato,
Rich Felker
Cc: Jiri Slaby, linux-serial, linux-renesas-soc, uclinux-h8-devel,
linux-sh, Geert Uytterhoeven
In-Reply-To: <1512047522-16312-1-git-send-email-geert+renesas@glider.be>
Renesas H8/300 and ARM platforms use DT and support earlycon, so most
users want earlycon support to be enabled.
On SuperH platforms, earlycon is not yet supported.
Hence follow the above rationale to configure the default, unless
CONFIG_EXPERT is enabled.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 0c75562d620feb82..952a2c6a9da08fdd 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -774,10 +774,11 @@ config SERIAL_SH_SCI_CONSOLE
default y
config SERIAL_SH_SCI_EARLYCON
- bool "Support for early console on SuperH SCI(F)"
+ bool "Support for early console on SuperH SCI(F)" if EXPERT
depends on SERIAL_SH_SCI=y
select SERIAL_CORE_CONSOLE
select SERIAL_EARLYCON
+ default ARCH_RENESAS || H8300
config SERIAL_SH_SCI_DMA
bool "DMA support"
--
2.7.4
^ permalink raw reply related
* [PATCH 2/5] tty: serial: sh-sci: Hide serial console config question
From: Geert Uytterhoeven @ 2017-11-30 13:11 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman, Magnus Damm, Yoshinori Sato,
Rich Felker
Cc: Jiri Slaby, linux-serial, linux-renesas-soc, uclinux-h8-devel,
linux-sh, Geert Uytterhoeven
In-Reply-To: <1512047522-16312-1-git-send-email-geert+renesas@glider.be>
Most users will want to use a serial console.
Hence make that the default, unless CONFIG_EXPERT is enabled.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index bd96046d7f94bac0..0c75562d620feb82 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -768,9 +768,10 @@ config SERIAL_SH_SCI_NR_UARTS
default "18" if ARCH_RENESAS
config SERIAL_SH_SCI_CONSOLE
- bool "Support for console on SuperH SCI(F)"
+ bool "Support for console on SuperH SCI(F)" if EXPERT
depends on SERIAL_SH_SCI=y
select SERIAL_CORE_CONSOLE
+ default y
config SERIAL_SH_SCI_EARLYCON
bool "Support for early console on SuperH SCI(F)"
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] tty: serial: sh-sci: Hide number of ports config question
From: Geert Uytterhoeven @ 2017-11-30 13:11 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman, Magnus Damm, Yoshinori Sato,
Rich Felker
Cc: Jiri Slaby, linux-serial, linux-renesas-soc, uclinux-h8-devel,
linux-sh, Geert Uytterhoeven
In-Reply-To: <1512047522-16312-1-git-send-email-geert+renesas@glider.be>
Auto-configure the maximum number of serial ports based on how many can
be present on the architecture:
- 3 on H8/300,
- 10 on SuperH,
- 18 on Reneas ARM.
The default can still be overridden if CONFIG_EXPERT is enabled.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/Kconfig | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index b788fee54249deff..bd96046d7f94bac0 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -761,9 +761,11 @@ config SERIAL_SH_SCI
select SERIAL_MCTRL_GPIO if GPIOLIB
config SERIAL_SH_SCI_NR_UARTS
- int "Maximum number of SCI(F) serial ports"
+ int "Maximum number of SCI(F) serial ports" if EXPERT
depends on SERIAL_SH_SCI
- default "2"
+ default "3" if H8300
+ default "10" if SUPERH
+ default "18" if ARCH_RENESAS
config SERIAL_SH_SCI_CONSOLE
bool "Support for console on SuperH SCI(F)"
--
2.7.4
^ permalink raw reply related
* [PATCH 0/5] tty: serial: sh-sci: Hide driver specific config questions
From: Geert Uytterhoeven @ 2017-11-30 13:11 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman, Magnus Damm, Yoshinori Sato,
Rich Felker
Cc: Jiri Slaby, linux-serial, linux-renesas-soc, uclinux-h8-devel,
linux-sh, Geert Uytterhoeven
Hi Greg, Simon, Magnus, Sato-san, Rich,
The Renesas (H)SCI(F) driver has several driver-specific config options.
This may confuse users, and leads to inefficient testing and development.
E.g. recently a regression was introduced in SCIF DMA support, and
someone didn't see early con output, both due to the relevant options
being accidentally disabled.
This patch series is an attempt to stop bothering (H)SCI(F) users with
several config questions, while providing sensible defaults for these
options, based on the target platform. The defaults can still be
overridden if CONFIG_EXPERT is enabled.
The last patch updates shmobile_defconfig to take into account the
defaults. It is marked RFC, as it depends on the first 4 patches to be
effective, and to avoid feature regressions.
Thanks for your comments!
Geert Uytterhoeven (5):
tty: serial: sh-sci: Hide number of ports config question
tty: serial: sh-sci: Hide serial console config question
tty: serial: sh-sci: Hide earlycon config question
tty: serial: sh-sci: Hide DMA config question
[RFC] ARM: shmobile: defconfig: Disable CONFIG_EMBEDDED
arch/arm/configs/shmobile_defconfig | 6 +-----
drivers/tty/serial/Kconfig | 15 ++++++++++-----
2 files changed, 11 insertions(+), 10 deletions(-)
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 27/35] irqchip: Andestech Internal Vector Interrupt Controller driver
From: Marc Zyngier @ 2017-11-30 10:57 UTC (permalink / raw)
To: Greentime Hu
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Rob Herring, netdev, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial, Rick Chen
In-Reply-To: <CAEbi=3eXP1cfNDhu9YjZbjPghgxm9xmEtjEBO=KVOnWZyPqVbw@mail.gmail.com>
On Wed, Nov 29 2017 at 11:23:34 pm GMT, Greentime Hu <green.hu@gmail.com> wrote:
Hi Greentime,
>>> +}
>>> +
>>> +static void ativic32_mask_ack_irq(struct irq_data *data)
>>> +{
>>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)),
>>> NDS32_SR_INT_MASK2);
>>> + __nds32__mtsr_dsb((1 << data->hwirq), NDS32_SR_INT_PEND2);
>>
>> This is effectively MASK+ACK, so you're better off just writing it as
>> such. And since there is no advantage in your implementation in having
>> MASK_ACK over MASK+ACK, I suggest you remove this function completely,
>> and rely on the core code which will call them in sequence.
>
> I think mask_ack is still better than mask + ack because we don't need
> to do two function call.
> We can save a prologue and a epilogue. It will benefit interrupt latency.
Can you actually measure this? Your CPU would have to be extremely slow
if you could see the impact of an extra function call on interrupt
latency. From a maintenance perspective, this isn't very nice (it is
effectively code duplication).
I suggest you start with the simplest version of the code, and provide
an optimisation once you can measurably demonstrate that mask_ack is
better.
[...]
>>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_edge_irq);
>>> + else
>>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_level_irq);
>>
>> Since you do not express the trigger in DT, you need to tell the core
>> about it by calling irqd_set_trigger_type() with the right setting.
>>
>
> Since the comments say so, I will add ativic32_set_type() for irq_set_type()
> in the next version patch.
>
> /*
> * Must only be called inside irq_chip.irq_set_type() functions.
> */
> static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)
> {
> __irqd_to_state(d) &= ~IRQD_TRIGGER_MASK;
> __irqd_to_state(d) |= type & IRQD_TRIGGER_MASK;
> }
>
> It will be like this.
> static int ativic32_set_type(struct irq_data *data, unsigned int flow_type)
> {
> irqd_set_trigger_type(data, flow_type);
> return IRQ_SET_MASK_OK;
> }
This feels wrong. Your interrupt controller doesn't seem to support the
trigger being changed, so irq_set_type would be a bit pointless. A
driver trying to set a level trigger on an edge interrupt would succeed,
and that is as bad as it gets. All you need is to expose the capability
of the HW when you register the flow handler instead of adding a rather
misleading callback.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* RE: [patch v12 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Oleksandr Shamray @ 2017-11-30 10:49 UTC (permalink / raw)
To: Philippe Ombredanne, Kun Yi
Cc: Greg Kroah-Hartman, arnd@arndb.de, system-sw-low-level,
devicetree@vger.kernel.org, jiri@resnulli.us, Vadim Pasternak,
linux-api@vger.kernel.org, OpenBMC Maillist, LKML,
openocd-devel-owner@lists.sourceforge.net, mec@shout.net,
Jiri Pirko, robh+dt@kernel.org, linux-serial@vger.kernel.org,
tklauser@distanz.ch, Mauro Carvalho Chehab, Dav
In-Reply-To: <CAOFm3uHg8n4A8opaOC3=pNtM1yJU6iy5Ta8XtGOvqT-Euv=X1w@mail.gmail.com>
Thnaks for point
Best Regards,
Oleksandr Shamray
> -----Original Message-----
> From: Philippe Ombredanne [mailto:pombredanne@nexb.com]
> Sent: Thursday, November 30, 2017 10:21 AM
> To: Kun Yi <kunyi@google.com>
> Cc: Oleksandr Shamray <oleksandrs@mellanox.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; arnd@arndb.de; system-sw-low-level <system-
> sw-low-level@mellanox.com>; devicetree@vger.kernel.org; jiri@resnulli.us;
> Vadim Pasternak <vadimp@mellanox.com>; linux-api@vger.kernel.org;
> OpenBMC Maillist <openbmc@lists.ozlabs.org>; LKML <linux-
> kernel@vger.kernel.org>; openocd-devel-owner@lists.sourceforge.net;
> mec@shout.net; Jiri Pirko <jiri@mellanox.com>; robh+dt@kernel.org; linux-
> serial@vger.kernel.org; tklauser@distanz.ch; Mauro Carvalho Chehab
> <mchehab@kernel.org>; David S. Miller <davem@davemloft.net>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [patch v12 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx
> families JTAG master driver
>
> On Wed, Nov 29, 2017 at 11:51 PM, Kun Yi <kunyi@google.com> wrote:
> > On Tue, Nov 14, 2017 at 8:11 AM, Oleksandr Shamray
> > <oleksandrs@mellanox.com> wrote:
> []
> >> diff --git a/drivers/jtag/jtag-aspeed.c b/drivers/jtag/jtag-aspeed.c
> >> new file mode 100644 index 0000000..a6e2417
> >> --- /dev/null
> >> +++ b/drivers/jtag/jtag-aspeed.c
> >> @@ -0,0 +1,782 @@
> >> +/*
> >> + * drivers/jtag/aspeed-jtag.c
> >> + *
> >> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> >> + * Copyright (c) 2017 Oleksandr Shamray <oleksandrs@mellanox.com>
> >> + *
> >> + * Released under the GPLv2 only.
> >> + * SPDX-License-Identifier: GPL-2.0
> >> + */
>
> I think the SPDX id should be on the first line as requested by Linus and
> documented by Thomas (tglx) and Greg (greg-kh). And it should use //
> comments, e.g:
>
> // SPDX-License-Identifier: GPL-2.0
>
> See the threads discussing all these.
>
>
> --
> Cordially
> Philippe Ombredanne
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Greentime Hu @ 2017-11-30 10:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano,
linux-serial@vger.kernel.org, Vincent Chen
In-Reply-To: <CAK8P3a3-wXAX+bRaDxs8+N5u6d-O6_Uwg7Wzp9m9=tSqQpPrEw@mail.gmail.com>
2017-11-30 17:30 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Thu, Nov 30, 2017 at 6:48 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>
>>> When you put them in a sorted list like I mentioned for simplicity, you
>>> could reduce the confusion by naming them differently, e.g.
>>> CONFIG_CPU_N10_OR_NEWER.
>>>
>>> Having only the CPU_CACHE_NONALIASING option is fine if you
>>> never need to make any other decisions based on the CPU core
>>> type, but then the help text should describe specifically which cases
>>> are affected (N10/N13/D13 with 4K page size), and you can decide to
>>> hide the option and make it always-on when using 8K page size.
>>>
>>> Arnd
>>
>>
>> Hi, Arnd:
>>
>> I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
>> It will be implemented like this.
>
> I think I'm still a bit confused about the relation between CPU cores
> and architecture levels. Is it correct to say that there are orthogonal,
> and that you can have e.g. an N10 core implementing either nds32v2
> or nds32v3?
>
Yup, we did having N10 cores are implementing either nds32v3 or
nds32v2, but nds32v2 are not used anymore.
We can assume every nds32 cores are v3.
> There is nothing wrong with that of course, it's just not what I
> expected from having worked with other architectures.
>
> I also see that GCC has no pipeline specific optimizations for
> specific cores, it just understands the differences between the
> architecture levels, so at least today there is way to pass e.g.
> "-march=nds32v2 -mtune=d15" to generate code that would
> work on both v2 and v3 but be optimized for d15.
Thanks. We will work on that.
>> config HWZOL
>> bool "hardware zero overhead loop support"
>> depends on CPU_D10 || CPU_D15
>> default n
>> help
>> A set of Zero-Overhead Loop mechanism is provided to reduce the
>> instruction fetch and execution overhead of loop-control instructions.
>> It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
>> You don't need to save these registers if you can make sure your user
>> program doesn't use these registers.
>>
>> If unsure, say N.
>>
>> config CPU_CACHE_NONALIASING
>> bool "Non-aliasing cache"
>> depends on !CPU_N10 && !CPU_D10
>> default n
>> help
>> If this CPU is using VIPT data cache and its cache way size is larger
>> than page size, say N. If it is using PIPT data cache, say Y.
>>
>> If unsure, say N.
>
> This looks ok, yes, but as Geert said, it would seem more intuitive to
> write it as
>
> config CPU_CACHE_ALIASING
> bool "Aliasing VIPT cache"
> depends on CPU_N10 || CPU_D10
>
>> choice
>> prompt "CPU type"
>> default CPU_V3
>> config CPU_N15
>> bool "AndesCore N15"
>> select CPU_CACHE_NONALIASING
>> config CPU_N13
>> bool "AndesCore N13"
>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>> config CPU_N10
>> bool "AndesCore N10"
>> config CPU_D15
>> bool "AndesCore D15"
>> select CPU_CACHE_NONALIASING
>> config CPU_D10
>> bool "AndesCore D10"
>> config CPU_V3
>> bool "AndesCore v3 compatible"
>> select ANDES_PAGE_SIZE_4KB
>> endchoice
>
> Two points here:
>
> - Generally you should not mix 'select' and 'depends on' like this.
> Either you make the cache aliasing a user visible option that
> uses 'depends on' with a combination of CPU cores, or you
> make it a hidden option (with no string after the "bool" keyword)
> that always gets selected from the per-cpu options.
>
> - There is a little-known trick with choice statements that allows
> you to use 'tristate' instead of 'bool' in the choice. In that case,
> you can enable multiple options together as long as all of them
> are 'm'.
>
> Arnd
Thanks.
CPU_CACHE_ALIASING is more intuitive. I will apply it.
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Arnd Bergmann @ 2017-11-30 9:30 UTC (permalink / raw)
To: Greentime Hu
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Vincent Chen
In-Reply-To: <CAEbi=3cTkbt9i7XPXMnY1D6qtbebDW1x8sFVsgqhq-nApAx5mA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Nov 30, 2017 at 6:48 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>> When you put them in a sorted list like I mentioned for simplicity, you
>> could reduce the confusion by naming them differently, e.g.
>> CONFIG_CPU_N10_OR_NEWER.
>>
>> Having only the CPU_CACHE_NONALIASING option is fine if you
>> never need to make any other decisions based on the CPU core
>> type, but then the help text should describe specifically which cases
>> are affected (N10/N13/D13 with 4K page size), and you can decide to
>> hide the option and make it always-on when using 8K page size.
>>
>> Arnd
>
>
> Hi, Arnd:
>
> I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
> It will be implemented like this.
I think I'm still a bit confused about the relation between CPU cores
and architecture levels. Is it correct to say that there are orthogonal,
and that you can have e.g. an N10 core implementing either nds32v2
or nds32v3?
There is nothing wrong with that of course, it's just not what I
expected from having worked with other architectures.
I also see that GCC has no pipeline specific optimizations for
specific cores, it just understands the differences between the
architecture levels, so at least today there is way to pass e.g.
"-march=nds32v2 -mtune=d15" to generate code that would
work on both v2 and v3 but be optimized for d15.
> config HWZOL
> bool "hardware zero overhead loop support"
> depends on CPU_D10 || CPU_D15
> default n
> help
> A set of Zero-Overhead Loop mechanism is provided to reduce the
> instruction fetch and execution overhead of loop-control instructions.
> It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
> You don't need to save these registers if you can make sure your user
> program doesn't use these registers.
>
> If unsure, say N.
>
> config CPU_CACHE_NONALIASING
> bool "Non-aliasing cache"
> depends on !CPU_N10 && !CPU_D10
> default n
> help
> If this CPU is using VIPT data cache and its cache way size is larger
> than page size, say N. If it is using PIPT data cache, say Y.
>
> If unsure, say N.
This looks ok, yes, but as Geert said, it would seem more intuitive to
write it as
config CPU_CACHE_ALIASING
bool "Aliasing VIPT cache"
depends on CPU_N10 || CPU_D10
> choice
> prompt "CPU type"
> default CPU_V3
> config CPU_N15
> bool "AndesCore N15"
> select CPU_CACHE_NONALIASING
> config CPU_N13
> bool "AndesCore N13"
> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
> config CPU_N10
> bool "AndesCore N10"
> config CPU_D15
> bool "AndesCore D15"
> select CPU_CACHE_NONALIASING
> config CPU_D10
> bool "AndesCore D10"
> config CPU_V3
> bool "AndesCore v3 compatible"
> select ANDES_PAGE_SIZE_4KB
> endchoice
Two points here:
- Generally you should not mix 'select' and 'depends on' like this.
Either you make the cache aliasing a user visible option that
uses 'depends on' with a combination of CPU cores, or you
make it a hidden option (with no string after the "bool" keyword)
that always gets selected from the per-cpu options.
- There is a little-known trick with choice statements that allows
you to use 'tristate' instead of 'bool' in the choice. In that case,
you can enable multiple options together as long as all of them
are 'm'.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Greentime Hu @ 2017-11-30 9:29 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Greentime, Linux Kernel Mailing List, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Networking, Vincent Chen, DTML, Al Viro, David Howells,
Will Deacon, Daniel Lezcano, linux-serial@vger.kernel.org,
Vincent Chen
In-Reply-To: <CAMuHMdWXe-1=-98V=AcUzu-MiEaAqPQi2HZ6OTS=8sXY3XNhjw@mail.gmail.com>
2017-11-30 15:52 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Thu, Nov 30, 2017 at 6:48 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@gmail.com> wrote:
>> I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
>> It will be implemented like this.
>>
>> config HWZOL
>> bool "hardware zero overhead loop support"
>> depends on CPU_D10 || CPU_D15
>> default n
>> help
>> A set of Zero-Overhead Loop mechanism is provided to reduce the
>> instruction fetch and execution overhead of loop-control instructions.
>> It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
>> You don't need to save these registers if you can make sure your user
>> program doesn't use these registers.
>>
>> If unsure, say N.
>>
>> config CPU_CACHE_NONALIASING
>> bool "Non-aliasing cache"
>> depends on !CPU_N10 && !CPU_D10
>> default n
>> help
>> If this CPU is using VIPT data cache and its cache way size is larger
>> than page size, say N. If it is using PIPT data cache, say Y.
>>
>> If unsure, say N.
>
> I still think it will be easier to revert the logic, and have
> CPU_CACHE_ALIASING.
>
Thanks Geert
I will implement it like this.
config HWZOL
bool "hardware zero overhead loop support"
depends on CPU_D10 || CPU_D15
default n
help
A set of Zero-Overhead Loop mechanism is provided to reduce the
instruction fetch and execution overhead of loop-control instructions.
It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
You don't need to save these registers if you can make sure your user
program doesn't use these registers.
If unsure, say N.
config CPU_CACHE_ALIASING
bool "Aliasing cache"
depends on CPU_N10 || CPU_D10 || CPU_N13 || CPU_V3
default y
help
If this CPU is using VIPT data cache and its cache way size is larger
than page size, say Y. If it is using PIPT data cache, say N.
If unsure, say Y.
choice
prompt "CPU type"
default CPU_V3
config CPU_N15
bool "AndesCore N15"
config CPU_N13
bool "AndesCore N13"
select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
config CPU_N10
bool "AndesCore N10"
select CPU_CACHE_ALIASING
config CPU_D15
bool "AndesCore D15"
config CPU_D10
bool "AndesCore D10"
select CPU_CACHE_ALIASING
config CPU_V3
bool "AndesCore v3 compatible"
select ANDES_PAGE_SIZE_8KB
endchoice
^ permalink raw reply
* Re: [patch v12 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Philippe Ombredanne @ 2017-11-30 8:21 UTC (permalink / raw)
To: Kun Yi
Cc: Oleksandr Shamray, Greg Kroah-Hartman, arnd-r2nGTMty4D4,
system-sw-low-level-VPRAkNaXOzVWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA, jiri-rHqAuBHg3fBzbRFIqnYvSA,
vadimp-VPRAkNaXOzVWk0Htik3J/w, linux-api-u79uwXL29TY76Z2rM5mHXA,
OpenBMC Maillist, LKML,
openocd-devel-owner-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
mec-WqBc5aa1uDFeoWH0uzbU5w, Jiri Pirko,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
tklauser-93Khv+1bN0NyDzI6CaY1VQ, Mauro Carvalho Chehab,
David S. Miller,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAGMNF6XRwU5QEJhkg-mVwE5zpPigtcGJtRUupZHtcWRdedcsxg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Nov 29, 2017 at 11:51 PM, Kun Yi <kunyi-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Nov 14, 2017 at 8:11 AM, Oleksandr Shamray
> <oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
[]
>> diff --git a/drivers/jtag/jtag-aspeed.c b/drivers/jtag/jtag-aspeed.c
>> new file mode 100644
>> index 0000000..a6e2417
>> --- /dev/null
>> +++ b/drivers/jtag/jtag-aspeed.c
>> @@ -0,0 +1,782 @@
>> +/*
>> + * drivers/jtag/aspeed-jtag.c
>> + *
>> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
>> + * Copyright (c) 2017 Oleksandr Shamray <oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> + *
>> + * Released under the GPLv2 only.
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
I think the SPDX id should be on the first line as requested by Linus
and documented by Thomas (tglx) and Greg (greg-kh). And it should use
// comments, e.g:
// SPDX-License-Identifier: GPL-2.0
See the threads discussing all these.
--
Cordially
Philippe Ombredanne
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Geert Uytterhoeven @ 2017-11-30 7:52 UTC (permalink / raw)
To: Greentime Hu
Cc: Arnd Bergmann, Greentime, Linux Kernel Mailing List, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Networking, Vincent Chen, DTML, Al Viro, David Howells,
Will Deacon, Daniel Lezcano, linux-serial@vger.kernel.org,
Vincent Chen
In-Reply-To: <CAEbi=3cTkbt9i7XPXMnY1D6qtbebDW1x8sFVsgqhq-nApAx5mA@mail.gmail.com>
On Thu, Nov 30, 2017 at 6:48 AM, Greentime Hu <green.hu@gmail.com> wrote:
> 2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@gmail.com> wrote:
> I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
> It will be implemented like this.
>
> config HWZOL
> bool "hardware zero overhead loop support"
> depends on CPU_D10 || CPU_D15
> default n
> help
> A set of Zero-Overhead Loop mechanism is provided to reduce the
> instruction fetch and execution overhead of loop-control instructions.
> It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
> You don't need to save these registers if you can make sure your user
> program doesn't use these registers.
>
> If unsure, say N.
>
> config CPU_CACHE_NONALIASING
> bool "Non-aliasing cache"
> depends on !CPU_N10 && !CPU_D10
> default n
> help
> If this CPU is using VIPT data cache and its cache way size is larger
> than page size, say N. If it is using PIPT data cache, say Y.
>
> If unsure, say N.
I still think it will be easier to revert the logic, and have
CPU_CACHE_ALIASING.
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: jsm_tty: Fix a possible null pointer dereference in two functions
From: Jiri Slaby @ 2017-11-30 6:41 UTC (permalink / raw)
To: SF Markus Elfring, Greg Kroah-Hartman, linux-serial
Cc: Guilherme G. Piccoli, Joe Perches, kernel-janitors, LKML
In-Reply-To: <089a4b85-64e8-cb4a-c5f8-9abb2556e5e5@users.sourceforge.net>
On 11/29/2017, 07:19 PM, SF Markus Elfring wrote:
>>> It's pretty unlikely, but it is an actual defect.
>>
>> No it is not, those variables will never be set to NULL,
>> so this can never be triggered. Walk up the call chain.
>
> If the involved software developers are convinced about the validity
> of this pointer:
>
> How do you think about to delete the following condition check
> instead in the discussed function implementations?
>
> if (!ch)
> return;
ACK
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Greentime Hu @ 2017-11-30 5:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano,
linux-serial@vger.kernel.org, Vincent Chen
In-Reply-To: <CAK8P3a3-CexhRkJB7_8TGShuDQP8w=myhkoB8-7ax3my96PrMw@mail.gmail.com>
2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>
>>>> How about this?
>>>>
>>>> choice
>>>> prompt "CPU type"
>>>> default CPU_N13
>>>> config CPU_N15
>>>> bool "AndesCore N15"
>>>> select CPU_CACHE_NONALIASING
>>>> config CPU_N13
>>>> bool "AndesCore N13"
>>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>>> config CPU_N10
>>>> bool "AndesCore N10"
>>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>>> config CPU_D15
>>>> bool "AndesCore D15"
>>>> select CPU_CACHE_NONALIASING
>>>> select HWZOL
>>>> config CPU_D10
>>>> bool "AndesCore D10"
>>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>>> endchoice
>>>
>>> With a 'choice' statement this works, but I would consider that
>>> suboptimal for another reason: now you cannot build a kernel that
>>> works on e.g. both N13 and N15.
>>>
>>> This is what we had on ARM a long time ago and on MIPS not so long
>>> ago, but it's really a burden for testing and distribution once you get
>>> support for more than a handful of machines supported in the mainline
>>> kernel: If each CPU core is mutually incompatible with the other ones,
>>> this means you likely end up having one defconfig per CPU core,
>>> or even one defconfig per SoC or board.
>>>
>>> I would always try to get the largest amount of hardware to work
>>> in the same kernel concurrently.
>>>
>>> One way of of this would be to define the "CPU type" as the minimum
>>> supported CPU, e.g. selecting D15 would result in a kernel that
>>> only works on D15, while selecting N15 would work on both N15 and
>>> D15, and selecting D10 would work on both D10 and D15.
>>>
>>
>> Hi, Arnd:
>>
>> Maybe we should keep the original implementation for this reason.
>> The default value of CPU_CACHE_NONALIASING and ANDES_PAGE_SIZE_8KB is
>> available for all CPU types for now.
>> User can use these configs built kernel to boot on almost all nds32 CPUs.
>>
>> It might be a little bit weird if we config CPU_N10 but run on a N13 CPU.
>> This might confuse our users.
>
> I think it really depends on how much flexibility you want to give to users.
> The way I suggested first was to allow selecting an arbitrary combination
> of CPUs, and then let Kconfig derive the correct set of optimization flags
> and other options that work with those. That is probably the easiest for
> the users, but can be tricky to get right for all combinations.
>
> When you put them in a sorted list like I mentioned for simplicity, you
> could reduce the confusion by naming them differently, e.g.
> CONFIG_CPU_N10_OR_NEWER.
>
> Having only the CPU_CACHE_NONALIASING option is fine if you
> never need to make any other decisions based on the CPU core
> type, but then the help text should describe specifically which cases
> are affected (N10/N13/D13 with 4K page size), and you can decide to
> hide the option and make it always-on when using 8K page size.
>
> Arnd
Hi, Arnd:
I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
It will be implemented like this.
config HWZOL
bool "hardware zero overhead loop support"
depends on CPU_D10 || CPU_D15
default n
help
A set of Zero-Overhead Loop mechanism is provided to reduce the
instruction fetch and execution overhead of loop-control instructions.
It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
You don't need to save these registers if you can make sure your user
program doesn't use these registers.
If unsure, say N.
config CPU_CACHE_NONALIASING
bool "Non-aliasing cache"
depends on !CPU_N10 && !CPU_D10
default n
help
If this CPU is using VIPT data cache and its cache way size is larger
than page size, say N. If it is using PIPT data cache, say Y.
If unsure, say N.
choice
prompt "CPU type"
default CPU_V3
config CPU_N15
bool "AndesCore N15"
select CPU_CACHE_NONALIASING
config CPU_N13
bool "AndesCore N13"
select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
config CPU_N10
bool "AndesCore N10"
config CPU_D15
bool "AndesCore D15"
select CPU_CACHE_NONALIASING
config CPU_D10
bool "AndesCore D10"
config CPU_V3
bool "AndesCore v3 compatible"
select ANDES_PAGE_SIZE_4KB
endchoice
^ permalink raw reply
* Re: [patch v12 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Kun Yi @ 2017-11-29 22:51 UTC (permalink / raw)
To: Oleksandr Shamray
Cc: gregkh, arnd, system-sw-low-level, devicetree, jiri, vadimp,
linux-api, OpenBMC Maillist, linux-kernel, openocd-devel-owner,
mec, Jiri Pirko, robh+dt, linux-serial, tklauser, mchehab, davem,
linux-arm-kernel
In-Reply-To: <1510675867-24276-3-git-send-email-oleksandrs@mellanox.com>
Thanks for working on the driver, Oleksandr. I gave this a try on a
board with Aspeed 2520. One question below:
On Tue, Nov 14, 2017 at 8:11 AM, Oleksandr Shamray
<oleksandrs@mellanox.com> wrote:
> Driver adds support of Aspeed 2500/2400 series SOC JTAG master controller.
>
> Driver implements the following jtag ops:
> - freq_get;
> - freq_set;
> - status_get;
> - idle;
> - xfer;
>
> It has been tested on Mellanox system with BMC equipped with
> Aspeed 2520 SoC for programming CPLD devices.
>
> Signed-off-by: Oleksandr Shamray <oleksandrs@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v11->v12
> Comments pointed by Chip Bilbrey <chip@bilbrey.org>
> - Remove access mode from xfer and idle transactions
> - Add new ioctl JTAG_SIOCMODE for set hw mode
>
> v10->v11
> v9->v10
> V8->v9
> Comments pointed by Arnd Bergmann <arnd@arndb.de>
> - add *data parameter to xfer function prototype
>
> v7->v8
> Comments pointed by Joel Stanley <joel.stan@gmail.com>
> - aspeed_jtag_init replace goto to return;
> - change input variables type from __u32 to u32
> in functios freq_get, freq_set, status_get
> - change sm_ variables type from char to u8
> - in jatg_init add disable clocks on error case
> - remove release_mem_region on error case
> - remove devm_free_irq on jtag_deinit
> - Fix misspelling Disabe/Disable
> - Change compatible string to ast2400 and ast2000
>
> v6->v7
> Notifications from kbuild test robot <lkp@intel.com>
> - Add include <linux/types.h> to jtag-asapeed.c
>
> v5->v6
> v4->v5
> Comments pointed by Arnd Bergmann <arnd@arndb.de>
> - Added HAS_IOMEM dependence in Kconfig to avoid
> "undefined reference to `devm_ioremap_resource'" error,
> because in some arch this not supported
>
> v3->v4
> Comments pointed by Arnd Bergmann <arnd@arndb.de>
> - change transaction pointer tdio type to __u64
> - change internal status type from enum to __u32
>
> v2->v3
>
> v1->v2
> Comments pointed by Greg KH <gregkh@linuxfoundation.org>
> - change license type from GPLv2/BSD to GPLv2
>
> Comments pointed by Neil Armstrong <narmstrong@baylibre.com>
> - Add clk_prepare_enable/clk_disable_unprepare in clock init/deinit
> - Change .compatible to soc-specific compatible names
> aspeed,aspeed4000-jtag/aspeed5000-jtag
> - Added dt-bindings
>
> Comments pointed by Arnd Bergmann <arnd@arndb.de>
> - Reorder functions and removed the forward declarations
> - Add static const qualifier to state machine states transitions
> - Change .compatible to soc-specific compatible names
> aspeed,aspeed4000-jtag/aspeed5000-jtag
> - Add dt-bindings
>
> Comments pointed by Randy Dunlap <rdunlap@infradead.org>
> - Change module name jtag-aspeed in description in Kconfig
>
> Comments pointed by kbuild test robot <lkp@intel.com>
> - Remove invalid include <asm/mach-types.h>
> - add resource_size instead of calculation
> ---
> drivers/jtag/Kconfig | 13 +
> drivers/jtag/Makefile | 1 +
> drivers/jtag/jtag-aspeed.c | 782 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 796 insertions(+), 0 deletions(-)
> create mode 100644 drivers/jtag/jtag-aspeed.c
>
> diff --git a/drivers/jtag/Kconfig b/drivers/jtag/Kconfig
> index 0fad1a3..098beb0 100644
> --- a/drivers/jtag/Kconfig
> +++ b/drivers/jtag/Kconfig
> @@ -14,3 +14,16 @@ menuconfig JTAG
>
> To compile this driver as a module, choose M here: the module will
> be called jtag.
> +
> +menuconfig JTAG_ASPEED
> + tristate "Aspeed SoC JTAG controller support"
> + depends on JTAG && HAS_IOMEM
> + ---help---
> + This provides a support for Aspeed JTAG device, equipped on
> + Aspeed SoC 24xx and 25xx families. Drivers allows programming
> + of hardware devices, connected to SoC through the JTAG interface.
> +
> + If you want this support, you should say Y here.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called jtag-aspeed.
> diff --git a/drivers/jtag/Makefile b/drivers/jtag/Makefile
> index af37493..04a855e 100644
> --- a/drivers/jtag/Makefile
> +++ b/drivers/jtag/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_JTAG) += jtag.o
> +obj-$(CONFIG_JTAG_ASPEED) += jtag-aspeed.o
> diff --git a/drivers/jtag/jtag-aspeed.c b/drivers/jtag/jtag-aspeed.c
> new file mode 100644
> index 0000000..a6e2417
> --- /dev/null
> +++ b/drivers/jtag/jtag-aspeed.c
> @@ -0,0 +1,782 @@
> +/*
> + * drivers/jtag/aspeed-jtag.c
> + *
> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2017 Oleksandr Shamray <oleksandrs@mellanox.com>
> + *
> + * Released under the GPLv2 only.
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/jtag.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <uapi/linux/jtag.h>
> +
> +#define ASPEED_JTAG_DATA 0x00
> +#define ASPEED_JTAG_INST 0x04
> +#define ASPEED_JTAG_CTRL 0x08
> +#define ASPEED_JTAG_ISR 0x0C
> +#define ASPEED_JTAG_SW 0x10
> +#define ASPEED_JTAG_TCK 0x14
> +#define ASPEED_JTAG_EC 0x18
> +
> +#define ASPEED_JTAG_DATA_MSB 0x01
> +#define ASPEED_JTAG_DATA_CHUNK_SIZE 0x20
> +
> +/* ASPEED_JTAG_CTRL: Engine Control */
> +#define ASPEED_JTAG_CTL_ENG_EN BIT(31)
> +#define ASPEED_JTAG_CTL_ENG_OUT_EN BIT(30)
> +#define ASPEED_JTAG_CTL_FORCE_TMS BIT(29)
> +#define ASPEED_JTAG_CTL_INST_LEN(x) ((x) << 20)
> +#define ASPEED_JTAG_CTL_LASPEED_INST BIT(17)
> +#define ASPEED_JTAG_CTL_INST_EN BIT(16)
> +#define ASPEED_JTAG_CTL_DR_UPDATE BIT(10)
> +#define ASPEED_JTAG_CTL_DATA_LEN(x) ((x) << 4)
> +#define ASPEED_JTAG_CTL_LASPEED_DATA BIT(1)
> +#define ASPEED_JTAG_CTL_DATA_EN BIT(0)
> +
> +/* ASPEED_JTAG_ISR : Interrupt status and enable */
> +#define ASPEED_JTAG_ISR_INST_PAUSE BIT(19)
> +#define ASPEED_JTAG_ISR_INST_COMPLETE BIT(18)
> +#define ASPEED_JTAG_ISR_DATA_PAUSE BIT(17)
> +#define ASPEED_JTAG_ISR_DATA_COMPLETE BIT(16)
> +#define ASPEED_JTAG_ISR_INST_PAUSE_EN BIT(3)
> +#define ASPEED_JTAG_ISR_INST_COMPLETE_EN BIT(2)
> +#define ASPEED_JTAG_ISR_DATA_PAUSE_EN BIT(1)
> +#define ASPEED_JTAG_ISR_DATA_COMPLETE_EN BIT(0)
> +#define ASPEED_JTAG_ISR_INT_EN_MASK GENMASK(3, 0)
> +#define ASPEED_JTAG_ISR_INT_MASK GENMASK(19, 16)
> +
> +/* ASPEED_JTAG_SW : Software Mode and Status */
> +#define ASPEED_JTAG_SW_MODE_EN BIT(19)
> +#define ASPEED_JTAG_SW_MODE_TCK BIT(18)
> +#define ASPEED_JTAG_SW_MODE_TMS BIT(17)
> +#define ASPEED_JTAG_SW_MODE_TDIO BIT(16)
> +
> +/* ASPEED_JTAG_TCK : TCK Control */
> +#define ASPEED_JTAG_TCK_DIVISOR_MASK GENMASK(10, 0)
> +#define ASPEED_JTAG_TCK_GET_DIV(x) ((x) & ASPEED_JTAG_TCK_DIVISOR_MASK)
> +
> +/* ASPEED_JTAG_EC : Controller set for go to IDLE */
> +#define ASPEED_JTAG_EC_GO_IDLE BIT(0)
> +
> +#define ASPEED_JTAG_IOUT_LEN(len) (ASPEED_JTAG_CTL_ENG_EN |\
> + ASPEED_JTAG_CTL_ENG_OUT_EN |\
> + ASPEED_JTAG_CTL_INST_LEN(len))
> +
> +#define ASPEED_JTAG_DOUT_LEN(len) (ASPEED_JTAG_CTL_ENG_EN |\
> + ASPEED_JTAG_CTL_ENG_OUT_EN |\
> + ASPEED_JTAG_CTL_DATA_LEN(len))
> +
> +#define ASPEED_JTAG_TCK_WAIT 10
> +#define ASPEED_JTAG_RESET_CNTR 10
> +
> +#define ASPEED_JTAG_NAME "jtag-aspeed"
> +
> +struct aspeed_jtag {
> + void __iomem *reg_base;
> + struct device *dev;
> + struct clk *pclk;
> + enum jtag_endstate status;
> + int irq;
> + u32 flag;
> + wait_queue_head_t jtag_wq;
> + u32 mode;
> +};
> +
> +static char *end_status_str[] = {"idle", "ir pause", "drpause"};
> +
> +static u32 aspeed_jtag_read(struct aspeed_jtag *aspeed_jtag, u32 reg)
> +{
> + return readl(aspeed_jtag->reg_base + reg);
> +}
> +
> +static void
> +aspeed_jtag_write(struct aspeed_jtag *aspeed_jtag, u32 val, u32 reg)
> +{
> + writel(val, aspeed_jtag->reg_base + reg);
> +}
> +
> +static int aspeed_jtag_freq_set(struct jtag *jtag, u32 freq)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> + unsigned long apb_frq;
> + u32 tck_val;
> + u16 div;
> +
> + apb_frq = clk_get_rate(aspeed_jtag->pclk);
> + div = (apb_frq % freq == 0) ? (apb_frq / freq) - 1 : (apb_frq / freq);
> + tck_val = aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_TCK);
> + aspeed_jtag_write(aspeed_jtag,
> + (tck_val & ASPEED_JTAG_TCK_DIVISOR_MASK) | div,
> + ASPEED_JTAG_TCK);
> + return 0;
> +}
> +
> +static int aspeed_jtag_freq_get(struct jtag *jtag, u32 *frq)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> + u32 pclk;
> + u32 tck;
> +
> + pclk = clk_get_rate(aspeed_jtag->pclk);
> + tck = aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_TCK);
> + *frq = pclk / (ASPEED_JTAG_TCK_GET_DIV(tck) + 1);
> +
> + return 0;
> +}
> +
> +static int aspeed_jtag_mode_set(struct jtag *jtag, u32 mode)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> +
> + aspeed_jtag->mode = mode;
> + return 0;
> +}
> +
> +static void aspeed_jtag_sw_delay(struct aspeed_jtag *aspeed_jtag, int cnt)
> +{
> + int i;
> +
> + for (i = 0; i < cnt; i++)
> + aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_SW);
> +}
> +
> +static char aspeed_jtag_tck_cycle(struct aspeed_jtag *aspeed_jtag,
> + u8 tms, u8 tdi)
> +{
> + char tdo = 0;
> +
> + /* TCK = 0 */
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + (tms * ASPEED_JTAG_SW_MODE_TMS) |
> + (tdi * ASPEED_JTAG_SW_MODE_TDIO), ASPEED_JTAG_SW);
> +
> + aspeed_jtag_sw_delay(aspeed_jtag, ASPEED_JTAG_TCK_WAIT);
> +
> + /* TCK = 1 */
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TCK |
> + (tms * ASPEED_JTAG_SW_MODE_TMS) |
> + (tdi * ASPEED_JTAG_SW_MODE_TDIO), ASPEED_JTAG_SW);
> +
> + if (aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_SW) &
> + ASPEED_JTAG_SW_MODE_TDIO)
> + tdo = 1;
> +
> + aspeed_jtag_sw_delay(aspeed_jtag, ASPEED_JTAG_TCK_WAIT);
> +
> + /* TCK = 0 */
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + (tms * ASPEED_JTAG_SW_MODE_TMS) |
> + (tdi * ASPEED_JTAG_SW_MODE_TDIO), ASPEED_JTAG_SW);
> + return tdo;
> +}
> +
> +static void aspeed_jtag_wait_instruction_pause(struct aspeed_jtag *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> + ASPEED_JTAG_ISR_INST_PAUSE);
> + aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_INST_PAUSE;
> +}
> +
> +static void
> +aspeed_jtag_wait_instruction_complete(struct aspeed_jtag *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> + ASPEED_JTAG_ISR_INST_COMPLETE);
> + aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_INST_COMPLETE;
> +}
> +
> +static void
> +aspeed_jtag_wait_data_pause_complete(struct aspeed_jtag *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> + ASPEED_JTAG_ISR_DATA_PAUSE);
> + aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_DATA_PAUSE;
> +}
> +
> +static void aspeed_jtag_wait_data_complete(struct aspeed_jtag *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> + ASPEED_JTAG_ISR_DATA_COMPLETE);
> + aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_DATA_COMPLETE;
> +}
> +
> +static void aspeed_jtag_sm_cycle(struct aspeed_jtag *aspeed_jtag, const u8 *tms,
> + int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, tms[i], 0);
> +}
> +
> +static void aspeed_jtag_run_test_idle_sw(struct aspeed_jtag *aspeed_jtag,
> + struct jtag_run_test_idle *runtest)
> +{
> + static const u8 sm_pause_irpause[] = {1, 1, 1, 1, 0, 1, 0};
> + static const u8 sm_pause_drpause[] = {1, 1, 1, 0, 1, 0};
> + static const u8 sm_idle_irpause[] = {1, 1, 0, 1, 0};
> + static const u8 sm_idle_drpause[] = {1, 0, 1, 0};
> + static const u8 sm_pause_idle[] = {1, 1, 0};
> + int i;
> +
> + /* SW mode from idle/pause-> to pause/idle */
> + if (runtest->reset) {
> + for (i = 0; i < ASPEED_JTAG_RESET_CNTR; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, 1, 0);
> + }
> +
> + switch (aspeed_jtag->status) {
> + case JTAG_STATE_IDLE:
> + switch (runtest->endstate) {
> + case JTAG_STATE_PAUSEIR:
> + /* ->DRSCan->IRSCan->IRCap->IRExit1->PauseIR */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_idle_irpause,
> + sizeof(sm_idle_irpause));
> +
> + aspeed_jtag->status = JTAG_STATE_PAUSEIR;
> + break;
> + case JTAG_STATE_PAUSEDR:
> + /* ->DRSCan->DRCap->DRExit1->PauseDR */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_idle_drpause,
> + sizeof(sm_idle_drpause));
> +
> + aspeed_jtag->status = JTAG_STATE_PAUSEDR;
> + break;
> + case JTAG_STATE_IDLE:
> + /* IDLE */
> + aspeed_jtag_tck_cycle(aspeed_jtag, 0, 0);
> + aspeed_jtag->status = JTAG_STATE_IDLE;
> + break;
> + default:
> + break;
> + }
> + break;
> +
> + case JTAG_STATE_PAUSEIR:
> + /* Fall-through */
> + case JTAG_STATE_PAUSEDR:
> + /* From IR/DR Pause */
> + switch (runtest->endstate) {
> + case JTAG_STATE_PAUSEIR:
> + /*
> + * to Exit2 IR/DR->Updt IR/DR->DRSCan->IRSCan->IRCap->
> + * IRExit1->PauseIR
> + */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_pause_irpause,
> + sizeof(sm_pause_irpause));
> +
> + aspeed_jtag->status = JTAG_STATE_PAUSEIR;
> + break;
> + case JTAG_STATE_PAUSEDR:
> + /*
> + * to Exit2 IR/DR->Updt IR/DR->DRSCan->DRCap->
> + * DRExit1->PauseDR
> + */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_pause_drpause,
> + sizeof(sm_pause_drpause));
> + aspeed_jtag->status = JTAG_STATE_PAUSEDR;
> + break;
> + case JTAG_STATE_IDLE:
> + /* to Exit2 IR/DR->Updt IR/DR->IDLE */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_pause_idle,
> + sizeof(sm_pause_idle));
> + aspeed_jtag->status = JTAG_STATE_IDLE;
> + break;
> + default:
> + break;
> + }
> + break;
> +
> + default:
> + dev_err(aspeed_jtag->dev, "aspeed_jtag_run_test_idle error\n");
> + break;
> + }
> +
> + /* Stay on IDLE for at least TCK cycle */
> + for (i = 0; i < runtest->tck; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, 0, 0);
> +}
> +
> +/**
> + * aspeed_jtag_run_test_idle:
> + * JTAG reset: generates at least 9 TMS high and 1 TMS low to force
> + * devices into Run-Test/Idle State.
> + */
> +static int aspeed_jtag_idle(struct jtag *jtag,
> + struct jtag_run_test_idle *runtest)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> +
> + dev_dbg(aspeed_jtag->dev, "aspeed_jtag runtest, status:%d, mode:%s, state:%s, reset:%d, tck:%d\n",
> + aspeed_jtag->status,
> + aspeed_jtag->mode & JTAG_XFER_HW_MODE ? "HW" : "SW",
> + end_status_str[runtest->endstate], runtest->reset,
> + runtest->tck);
> +
> + if (!(aspeed_jtag->mode & JTAG_XFER_HW_MODE)) {
> + aspeed_jtag_run_test_idle_sw(aspeed_jtag, runtest);
> + return 0;
> + }
> +
> + /* Disable sw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_SW);
> + /* x TMS high + 1 TMS low */
> + if (runtest->reset)
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_CTL_ENG_EN |
> + ASPEED_JTAG_CTL_ENG_OUT_EN |
> + ASPEED_JTAG_CTL_FORCE_TMS, ASPEED_JTAG_CTRL);
> + else
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_EC_GO_IDLE,
> + ASPEED_JTAG_EC);
> +
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
> +
> + aspeed_jtag->status = JTAG_STATE_IDLE;
> + return 0;
> +}
> +
> +static void aspeed_jtag_xfer_sw(struct aspeed_jtag *aspeed_jtag,
> + struct jtag_xfer *xfer, unsigned long *data)
> +{
> + unsigned long remain_xfer = xfer->length;
> + unsigned long shift_bits = 0;
> + unsigned long index = 0;
> + unsigned long tdi;
> + char tdo;
> +
> + if (xfer->direction == JTAG_READ_XFER)
> + tdi = UINT_MAX;
> + else
> + tdi = data[index];
> +
> + while (remain_xfer > 1) {
> + tdo = aspeed_jtag_tck_cycle(aspeed_jtag, 0,
> + tdi & ASPEED_JTAG_DATA_MSB);
> + data[index] |= tdo << (shift_bits %
> + ASPEED_JTAG_DATA_CHUNK_SIZE);
> +
> + tdi >>= 1;
> + shift_bits++;
> + remain_xfer--;
> +
> + if (shift_bits % ASPEED_JTAG_DATA_CHUNK_SIZE == 0) {
> + dev_dbg(aspeed_jtag->dev, "R/W data[%lu]:%lx\n",
> + index, data[index]);
> +
> + tdo = 0;
> + index++;
> +
> + if (xfer->direction == JTAG_READ_XFER)
> + tdi = UINT_MAX;
> + else
> + tdi = data[index];
> + }
> + }
> +
> + tdo = aspeed_jtag_tck_cycle(aspeed_jtag, 1, tdi & ASPEED_JTAG_DATA_MSB);
> + data[index] |= tdo << (shift_bits % ASPEED_JTAG_DATA_CHUNK_SIZE);
> +}
> +
> +static void aspeed_jtag_xfer_push_data(struct aspeed_jtag *aspeed_jtag,
> + enum jtag_xfer_type type, u32 bits_len)
> +{
> + dev_dbg(aspeed_jtag->dev, "shift bits %d\n", bits_len);
> +
> + if (type == JTAG_SIR_XFER) {
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_IOUT_LEN(bits_len),
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_DOUT_LEN(bits_len) |
> + ASPEED_JTAG_CTL_INST_EN, ASPEED_JTAG_CTRL);
> + } else {
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_DOUT_LEN(bits_len),
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_DOUT_LEN(bits_len) |
> + ASPEED_JTAG_CTL_DATA_EN, ASPEED_JTAG_CTRL);
> + }
> +}
> +
> +static void aspeed_jtag_xfer_push_data_last(struct aspeed_jtag *aspeed_jtag,
> + enum jtag_xfer_type type,
> + u32 shift_bits,
> + enum jtag_endstate endstate)
> +{
> + if (endstate != JTAG_STATE_IDLE) {
> + if (type == JTAG_SIR_XFER) {
> + dev_dbg(aspeed_jtag->dev, "IR Keep Pause\n");
> +
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_IOUT_LEN(shift_bits),
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_IOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_INST_EN,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_wait_instruction_pause(aspeed_jtag);
> + } else {
> + dev_dbg(aspeed_jtag->dev, "DR Keep Pause\n");
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_DOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_DR_UPDATE,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_DOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_DR_UPDATE |
> + ASPEED_JTAG_CTL_DATA_EN,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_wait_data_pause_complete(aspeed_jtag);
> + }
> + } else {
> + if (type == JTAG_SIR_XFER) {
> + dev_dbg(aspeed_jtag->dev, "IR go IDLE\n");
> +
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_IOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_LASPEED_INST,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_IOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_LASPEED_INST |
> + ASPEED_JTAG_CTL_INST_EN,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_wait_instruction_complete(aspeed_jtag);
> + } else {
> + dev_dbg(aspeed_jtag->dev, "DR go IDLE\n");
> +
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_DOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_LASPEED_DATA,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + ASPEED_JTAG_DOUT_LEN(shift_bits) |
> + ASPEED_JTAG_CTL_LASPEED_DATA |
> + ASPEED_JTAG_CTL_DATA_EN,
> + ASPEED_JTAG_CTRL);
> + aspeed_jtag_wait_data_complete(aspeed_jtag);
> + }
> + }
> +}
> +
> +static void aspeed_jtag_xfer_hw(struct aspeed_jtag *aspeed_jtag,
> + struct jtag_xfer *xfer, unsigned long *data)
> +{
> + unsigned long remain_xfer = xfer->length;
> + unsigned long index = 0;
> + char shift_bits;
> + u32 data_reg;
> +
> + data_reg = xfer->type == JTAG_SIR_XFER ?
> + ASPEED_JTAG_INST : ASPEED_JTAG_DATA;
> + while (remain_xfer) {
> + if (xfer->direction == JTAG_WRITE_XFER) {
> + dev_dbg(aspeed_jtag->dev, "W dr->dr_data[%lu]:%lx\n",
> + index, data[index]);
> +
> + aspeed_jtag_write(aspeed_jtag, data[index], data_reg);
> + } else {
> + aspeed_jtag_write(aspeed_jtag, 0, data_reg);
> + }
> +
> + if (remain_xfer > ASPEED_JTAG_DATA_CHUNK_SIZE) {
> + shift_bits = ASPEED_JTAG_DATA_CHUNK_SIZE;
> +
> + /*
> + * Read bytes were not equals to column length
> + * and go to Pause-DR
> + */
> + aspeed_jtag_xfer_push_data(aspeed_jtag, xfer->type,
> + shift_bits);
> + } else {
> + /*
> + * Read bytes equals to column length =>
> + * Update-DR
> + */
> + shift_bits = remain_xfer;
> + aspeed_jtag_xfer_push_data_last(aspeed_jtag, xfer->type,
> + shift_bits,
> + xfer->endstate);
> + }
> +
> + if (xfer->direction == JTAG_READ_XFER) {
> + if (shift_bits < ASPEED_JTAG_DATA_CHUNK_SIZE) {
> + data[index] = aspeed_jtag_read(aspeed_jtag,
> + data_reg);
> +
> + data[index] >>= ASPEED_JTAG_DATA_CHUNK_SIZE -
> + shift_bits;
> + } else {
> + data[index] = aspeed_jtag_read(aspeed_jtag,
> + data_reg);
> + }
> + dev_dbg(aspeed_jtag->dev, "R dr->dr_data[%lu]:%lx\n",
> + index, data[index]);
> + }
> +
> + remain_xfer = remain_xfer - shift_bits;
> + index++;
> + dev_dbg(aspeed_jtag->dev, "remain_xfer %lu\n", remain_xfer);
> + }
> +}
> +
> +static int aspeed_jtag_xfer(struct jtag *jtag, struct jtag_xfer *xfer,
> + u8 *xfer_data)
> +{
> + static const u8 sm_update_shiftir[] = {1, 1, 0, 0};
> + static const u8 sm_update_shiftdr[] = {1, 0, 0};
> + static const u8 sm_pause_idle[] = {1, 1, 0};
> + static const u8 sm_pause_update[] = {1, 1};
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> + unsigned long *data = (unsigned long *)xfer_data;
> + unsigned long remain_xfer = xfer->length;
> + unsigned long offset;
> + char dbg_str[256];
> + int pos = 0;
> + int i;
> +
> + for (offset = 0, i = 0; offset < xfer->length;
> + offset += ASPEED_JTAG_DATA_CHUNK_SIZE, i++) {
> + pos += snprintf(&dbg_str[pos], sizeof(dbg_str) - pos,
> + "0x%08lx ", data[i]);
> + }
> +
> + dev_dbg(aspeed_jtag->dev, "aspeed_jtag %s %s xfer, mode:%s, END:%d, len:%lu, TDI[%s]\n",
> + xfer->type == JTAG_SIR_XFER ? "SIR" : "SDR",
> + xfer->direction == JTAG_READ_XFER ? "READ" : "WRITE",
> + aspeed_jtag->mode & JTAG_XFER_HW_MODE ? "HW" : "SW",
> + xfer->endstate, remain_xfer, dbg_str);
> +
> + if (!(aspeed_jtag->mode & JTAG_XFER_HW_MODE)) {
> + /* SW mode */
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
> +
> + if (aspeed_jtag->status != JTAG_STATE_IDLE) {
> + /*IR/DR Pause->Exit2 IR / DR->Update IR /DR */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_pause_update,
> + sizeof(sm_pause_update));
> + }
> +
> + if (xfer->type == JTAG_SIR_XFER)
> + /* ->IRSCan->CapIR->ShiftIR */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_update_shiftir,
> + sizeof(sm_update_shiftir));
> + else
> + /* ->DRScan->DRCap->DRShift */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_update_shiftdr,
> + sizeof(sm_update_shiftdr));
> +
> + aspeed_jtag_xfer_sw(aspeed_jtag, xfer, data);
> +
> + /* DIPause/DRPause */
> + aspeed_jtag_tck_cycle(aspeed_jtag, 0, 0);
> +
> + if (xfer->endstate == JTAG_STATE_IDLE) {
> + /* ->DRExit2->DRUpdate->IDLE */
> + aspeed_jtag_sm_cycle(aspeed_jtag, sm_pause_idle,
> + sizeof(sm_pause_idle));
> + }
> + } else {
> + /* hw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_SW);
> + aspeed_jtag_xfer_hw(aspeed_jtag, xfer, data);
> + }
> +
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
> + aspeed_jtag->status = xfer->endstate;
> + return 0;
> +}
> +
> +static int aspeed_jtag_status_get(struct jtag *jtag, u32 *status)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> +
> + *status = aspeed_jtag->status;
> + return 0;
> +}
> +
> +static irqreturn_t aspeed_jtag_interrupt(s32 this_irq, void *dev_id)
> +{
> + struct aspeed_jtag *aspeed_jtag = dev_id;
> + irqreturn_t ret;
> + u32 status;
> +
> + status = aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_ISR);
> + dev_dbg(aspeed_jtag->dev, "status %x\n", status);
> +
> + if (status & ASPEED_JTAG_ISR_INT_MASK) {
> + aspeed_jtag_write(aspeed_jtag,
> + (status & ASPEED_JTAG_ISR_INT_MASK)
> + | (status & ASPEED_JTAG_ISR_INT_EN_MASK),
> + ASPEED_JTAG_ISR);
> + aspeed_jtag->flag |= status & ASPEED_JTAG_ISR_INT_MASK;
> + }
> +
> + if (aspeed_jtag->flag) {
> + wake_up_interruptible(&aspeed_jtag->jtag_wq);
> + ret = IRQ_HANDLED;
> + } else {
> + dev_err(aspeed_jtag->dev, "aspeed_jtag irq status:%x\n",
> + status);
> + ret = IRQ_NONE;
> + }
> + return ret;
> +}
> +
> +int aspeed_jtag_init(struct platform_device *pdev,
> + struct aspeed_jtag *aspeed_jtag)
> +{
> + struct resource *res;
> + int err;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + aspeed_jtag->reg_base = devm_ioremap_resource(aspeed_jtag->dev, res);
> + if (IS_ERR(aspeed_jtag->reg_base))
> + return -ENOMEM;
> +
> + aspeed_jtag->pclk = devm_clk_get(aspeed_jtag->dev, NULL);
> + if (IS_ERR(aspeed_jtag->pclk)) {
> + dev_err(aspeed_jtag->dev, "devm_clk_get failed\n");
> + return PTR_ERR(aspeed_jtag->pclk);
> + }
> +
> + clk_prepare_enable(aspeed_jtag->pclk);
> +
> + aspeed_jtag->irq = platform_get_irq(pdev, 0);
> + if (aspeed_jtag->irq < 0) {
> + dev_err(aspeed_jtag->dev, "no irq specified\n");
> + err = -ENOENT;
> + goto clk_unprep;
> + }
> +
> + /* Enable clock */
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_CTL_ENG_EN |
> + ASPEED_JTAG_CTL_ENG_OUT_EN, ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
> +
> + err = devm_request_irq(aspeed_jtag->dev, aspeed_jtag->irq,
> + aspeed_jtag_interrupt, 0,
> + "aspeed-jtag", aspeed_jtag);
> + if (err) {
> + dev_err(aspeed_jtag->dev, "aspeed_jtag unable to get IRQ");
> + goto clk_unprep;
> + }
> + dev_dbg(&pdev->dev, "aspeed_jtag:IRQ %d.\n", aspeed_jtag->irq);
> +
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_ISR_INST_PAUSE |
> + ASPEED_JTAG_ISR_INST_COMPLETE |
> + ASPEED_JTAG_ISR_DATA_PAUSE |
> + ASPEED_JTAG_ISR_DATA_COMPLETE |
> + ASPEED_JTAG_ISR_INST_PAUSE_EN |
> + ASPEED_JTAG_ISR_INST_COMPLETE_EN |
> + ASPEED_JTAG_ISR_DATA_PAUSE_EN |
> + ASPEED_JTAG_ISR_DATA_COMPLETE_EN,
> + ASPEED_JTAG_ISR);
> +
> + aspeed_jtag->flag = 0;
> + aspeed_jtag->mode = 0;
> + init_waitqueue_head(&aspeed_jtag->jtag_wq);
> + return 0;
> +
> +clk_unprep:
> + clk_disable_unprepare(aspeed_jtag->pclk);
> + return err;
> +}
> +
> +int aspeed_jtag_deinit(struct platform_device *pdev,
> + struct aspeed_jtag *aspeed_jtag)
> +{
> + aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_ISR);
> + /* Disable clock */
> + aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_CTRL);
> + clk_disable_unprepare(aspeed_jtag->pclk);
> + return 0;
> +}
> +
> +static const struct jtag_ops aspeed_jtag_ops = {
> + .freq_get = aspeed_jtag_freq_get,
> + .freq_set = aspeed_jtag_freq_set,
> + .status_get = aspeed_jtag_status_get,
> + .idle = aspeed_jtag_idle,
> + .xfer = aspeed_jtag_xfer,
> + .mode_set = aspeed_jtag_mode_set
> +};
> +
> +static int aspeed_jtag_probe(struct platform_device *pdev)
> +{
> + struct aspeed_jtag *aspeed_jtag;
> + struct jtag *jtag;
> + int err;
> +
> + if (!of_device_is_compatible(pdev->dev.of_node, "aspeed,aspeed-jtag"))
Should this be "aspeed,ast2400-jtag"/"aspeed,ast2500-jtag" as
specified in the compatible string below?
> + return -ENOMEM;
> +
> + jtag = jtag_alloc(sizeof(*aspeed_jtag), &aspeed_jtag_ops);
> + if (!jtag)
> + return -ENODEV;
> +
> + platform_set_drvdata(pdev, jtag);
> + aspeed_jtag = jtag_priv(jtag);
> + aspeed_jtag->dev = &pdev->dev;
> +
> + /* Initialize device*/
> + err = aspeed_jtag_init(pdev, aspeed_jtag);
> + if (err)
> + goto err_jtag_init;
> +
> + /* Initialize JTAG core structure*/
> + err = jtag_register(jtag);
> + if (err)
> + goto err_jtag_register;
> +
> + return 0;
> +
> +err_jtag_register:
> + aspeed_jtag_deinit(pdev, aspeed_jtag);
> +err_jtag_init:
> + jtag_free(jtag);
> + return err;
> +}
> +
> +static int aspeed_jtag_remove(struct platform_device *pdev)
> +{
> + struct jtag *jtag;
> +
> + jtag = platform_get_drvdata(pdev);
> + aspeed_jtag_deinit(pdev, jtag_priv(jtag));
> + jtag_unregister(jtag);
> + jtag_free(jtag);
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_jtag_of_match[] = {
> + { .compatible = "aspeed,ast2400-jtag", },
> + { .compatible = "aspeed,ast2500-jtag", },
> + {}
> +};
> +
> +static struct platform_driver aspeed_jtag_driver = {
> + .probe = aspeed_jtag_probe,
> + .remove = aspeed_jtag_remove,
> + .driver = {
> + .name = ASPEED_JTAG_NAME,
> + .of_match_table = aspeed_jtag_of_match,
> + },
> +};
> +module_platform_driver(aspeed_jtag_driver);
> +
> +MODULE_AUTHOR("Oleksandr Shamray <oleksandrs@mellanox.com>");
> +MODULE_DESCRIPTION("ASPEED JTAG driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.1
>
--
Regards,
Kun
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Arnd Bergmann @ 2017-11-29 20:27 UTC (permalink / raw)
To: Greentime Hu
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano,
linux-serial@vger.kernel.org, Vincent Chen
In-Reply-To: <CAEbi=3fmXP=n_tS+-TSFhdht2G=VHOzVCVU=SRy4A6Ym_GToUQ@mail.gmail.com>
On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>
>>> How about this?
>>>
>>> choice
>>> prompt "CPU type"
>>> default CPU_N13
>>> config CPU_N15
>>> bool "AndesCore N15"
>>> select CPU_CACHE_NONALIASING
>>> config CPU_N13
>>> bool "AndesCore N13"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_N10
>>> bool "AndesCore N10"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_D15
>>> bool "AndesCore D15"
>>> select CPU_CACHE_NONALIASING
>>> select HWZOL
>>> config CPU_D10
>>> bool "AndesCore D10"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> endchoice
>>
>> With a 'choice' statement this works, but I would consider that
>> suboptimal for another reason: now you cannot build a kernel that
>> works on e.g. both N13 and N15.
>>
>> This is what we had on ARM a long time ago and on MIPS not so long
>> ago, but it's really a burden for testing and distribution once you get
>> support for more than a handful of machines supported in the mainline
>> kernel: If each CPU core is mutually incompatible with the other ones,
>> this means you likely end up having one defconfig per CPU core,
>> or even one defconfig per SoC or board.
>>
>> I would always try to get the largest amount of hardware to work
>> in the same kernel concurrently.
>>
>> One way of of this would be to define the "CPU type" as the minimum
>> supported CPU, e.g. selecting D15 would result in a kernel that
>> only works on D15, while selecting N15 would work on both N15 and
>> D15, and selecting D10 would work on both D10 and D15.
>>
>
> Hi, Arnd:
>
> Maybe we should keep the original implementation for this reason.
> The default value of CPU_CACHE_NONALIASING and ANDES_PAGE_SIZE_8KB is
> available for all CPU types for now.
> User can use these configs built kernel to boot on almost all nds32 CPUs.
>
> It might be a little bit weird if we config CPU_N10 but run on a N13 CPU.
> This might confuse our users.
I think it really depends on how much flexibility you want to give to users.
The way I suggested first was to allow selecting an arbitrary combination
of CPUs, and then let Kconfig derive the correct set of optimization flags
and other options that work with those. That is probably the easiest for
the users, but can be tricky to get right for all combinations.
When you put them in a sorted list like I mentioned for simplicity, you
could reduce the confusion by naming them differently, e.g.
CONFIG_CPU_N10_OR_NEWER.
Having only the CPU_CACHE_NONALIASING option is fine if you
never need to make any other decisions based on the CPU core
type, but then the help text should describe specifically which cases
are affected (N10/N13/D13 with 4K page size), and you can decide to
hide the option and make it always-on when using 8K page size.
Arnd
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Dan Carpenter @ 2017-11-29 19:16 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Greg Kroah-Hartman,
Guilherme G. Piccoli, Jiri Slaby, LKML, kernel-janitors
In-Reply-To: <1511976187.19952.65.camel@perches.com>
On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote:
> On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 29 Nov 2017 17:30:36 +0100
> >
> > Move two debug messages so that a null pointer access can not happen
> > for the variable "ch" in these functions.
>
> An actual defect fix!
>
> Here you could probably cc stable too.
>
These aren't real bugs because "ch" is never NULL.
regards,
dan carpenter
^ permalink raw reply
* Re: jsm_tty: Fix a possible null pointer dereference in two functions
From: SF Markus Elfring @ 2017-11-29 18:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial
Cc: Joe Perches, Guilherme G. Piccoli, Jiri Slaby, LKML,
kernel-janitors
In-Reply-To: <20171129180528.GA24705@kroah.com>
>> It's pretty unlikely, but it is an actual defect.
>
> No it is not, those variables will never be set to NULL,
> so this can never be triggered. Walk up the call chain.
If the involved software developers are convinced about the validity
of this pointer:
How do you think about to delete the following condition check
instead in the discussed function implementations?
if (!ch)
return;
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Joe Perches @ 2017-11-29 18:16 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli, Jiri Slaby,
LKML, kernel-janitors
In-Reply-To: <20171129180528.GA24705@kroah.com>
On Wed, 2017-11-29 at 18:05 +0000, Greg Kroah-Hartman wrote:
> On Wed, Nov 29, 2017 at 09:51:36AM -0800, Joe Perches wrote:
> > On Wed, 2017-11-29 at 17:35 +0000, Greg Kroah-Hartman wrote:
> > > On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote:
> > > > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> > > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > > Date: Wed, 29 Nov 2017 17:30:36 +0100
> > > > >
> > > > > Move two debug messages so that a null pointer access can not happen
> > > > > for the variable "ch" in these functions.
> > > >
> > > > An actual defect fix!
> > >
> > > Nope, not at all, this does not "fix" anything.
> >
> > Well, I believe it does in unusual cases like a
> > CONFIG_DYNAMIC_DEBUG when this is enabled by an
> > odd +p in the dynamic debug control file.
> >
> > > > Here you could probably cc stable too.
> > >
> > > Nope, not worth it.
> >
> > <shrug>
> >
> > It's pretty unlikely, but it is an actual defect.
>
> No it is not, those variables will never be set to NULL, so this can
> never be triggered. Walk up the call chain.
Right you are. Local analysis isn't enough.
The code could/should be removed, but it's not a defect.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Greg Kroah-Hartman @ 2017-11-29 18:05 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli, Jiri Slaby,
LKML, kernel-janitors
In-Reply-To: <1511977896.19952.69.camel@perches.com>
On Wed, Nov 29, 2017 at 09:51:36AM -0800, Joe Perches wrote:
> On Wed, 2017-11-29 at 17:35 +0000, Greg Kroah-Hartman wrote:
> > On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote:
> > > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Wed, 29 Nov 2017 17:30:36 +0100
> > > >
> > > > Move two debug messages so that a null pointer access can not happen
> > > > for the variable "ch" in these functions.
> > >
> > > An actual defect fix!
> >
> > Nope, not at all, this does not "fix" anything.
>
> Well, I believe it does in unusual cases like a
> CONFIG_DYNAMIC_DEBUG when this is enabled by an
> odd +p in the dynamic debug control file.
>
> > > Here you could probably cc stable too.
> >
> > Nope, not worth it.
>
> <shrug>
>
> It's pretty unlikely, but it is an actual defect.
No it is not, those variables will never be set to NULL, so this can
never be triggered. Walk up the call chain.
greg k-h
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Joe Perches @ 2017-11-29 17:51 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli, Jiri Slaby,
LKML, kernel-janitors
In-Reply-To: <20171129173504.GA20581@kroah.com>
On Wed, 2017-11-29 at 17:35 +0000, Greg Kroah-Hartman wrote:
> On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote:
> > On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Wed, 29 Nov 2017 17:30:36 +0100
> > >
> > > Move two debug messages so that a null pointer access can not happen
> > > for the variable "ch" in these functions.
> >
> > An actual defect fix!
>
> Nope, not at all, this does not "fix" anything.
Well, I believe it does in unusual cases like a
CONFIG_DYNAMIC_DEBUG when this is enabled by an
odd +p in the dynamic debug control file.
> > Here you could probably cc stable too.
>
> Nope, not worth it.
<shrug>
It's pretty unlikely, but it is an actual defect.
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Greg Kroah-Hartman @ 2017-11-29 17:35 UTC (permalink / raw)
To: Joe Perches
Cc: SF Markus Elfring, linux-serial, Guilherme G. Piccoli, Jiri Slaby,
LKML, kernel-janitors
In-Reply-To: <1511976187.19952.65.camel@perches.com>
On Wed, Nov 29, 2017 at 09:23:07AM -0800, Joe Perches wrote:
> On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 29 Nov 2017 17:30:36 +0100
> >
> > Move two debug messages so that a null pointer access can not happen
> > for the variable "ch" in these functions.
>
> An actual defect fix!
Nope, not at all, this does not "fix" anything.
> Here you could probably cc stable too.
Nope, not worth it.
greg k-h
^ permalink raw reply
* Re: [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: Joe Perches @ 2017-11-29 17:23 UTC (permalink / raw)
To: SF Markus Elfring, linux-serial, Greg Kroah-Hartman,
Guilherme G. Piccoli, Jiri Slaby
Cc: LKML, kernel-janitors
In-Reply-To: <5c78db97-88f5-8655-9a47-eeee0a043fba@users.sourceforge.net>
On Wed, 2017-11-29 at 17:40 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 29 Nov 2017 17:30:36 +0100
>
> Move two debug messages so that a null pointer access can not happen
> for the variable "ch" in these functions.
An actual defect fix!
Here you could probably cc stable too.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 669fef464468d3f02d60a5cf725fc097e03c5cb8 ("serial: jsm: Convert jsm_printk to jsm_dbg")
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/tty/serial/jsm/jsm_tty.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
> index 469927d37b41..a34eed7344e5 100644
> --- a/drivers/tty/serial/jsm/jsm_tty.c
> +++ b/drivers/tty/serial/jsm/jsm_tty.c
> @@ -521,11 +521,10 @@ void jsm_input(struct jsm_channel *ch)
> int s = 0;
> int i = 0;
>
> - jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
> -
> if (!ch)
> return;
>
> + jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
> port = &ch->uart_port.state->port;
> tp = port->tty;
>
> @@ -647,10 +646,10 @@ static void jsm_carrier(struct jsm_channel *ch)
> int virt_carrier = 0;
> int phys_carrier = 0;
>
> - jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
> if (!ch)
> return;
>
> + jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
> bd = ch->ch_bd;
>
> if (!bd)
^ permalink raw reply
* [PATCH] jsm_tty: Fix a possible null pointer dereference in two functions
From: SF Markus Elfring @ 2017-11-29 16:40 UTC (permalink / raw)
To: linux-serial, Greg Kroah-Hartman, Guilherme G. Piccoli,
Jiri Slaby, Joe Perches
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 29 Nov 2017 17:30:36 +0100
Move two debug messages so that a null pointer access can not happen
for the variable "ch" in these functions.
This issue was detected by using the Coccinelle software.
Fixes: 669fef464468d3f02d60a5cf725fc097e03c5cb8 ("serial: jsm: Convert jsm_printk to jsm_dbg")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/tty/serial/jsm/jsm_tty.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/jsm/jsm_tty.c b/drivers/tty/serial/jsm/jsm_tty.c
index 469927d37b41..a34eed7344e5 100644
--- a/drivers/tty/serial/jsm/jsm_tty.c
+++ b/drivers/tty/serial/jsm/jsm_tty.c
@@ -521,11 +521,10 @@ void jsm_input(struct jsm_channel *ch)
int s = 0;
int i = 0;
- jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
-
if (!ch)
return;
+ jsm_dbg(READ, &ch->ch_bd->pci_dev, "start\n");
port = &ch->uart_port.state->port;
tp = port->tty;
@@ -647,10 +646,10 @@ static void jsm_carrier(struct jsm_channel *ch)
int virt_carrier = 0;
int phys_carrier = 0;
- jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
if (!ch)
return;
+ jsm_dbg(CARR, &ch->ch_bd->pci_dev, "start\n");
bd = ch->ch_bd;
if (!bd)
--
2.15.0
^ permalink raw reply related
* Re: [PATCH v2 27/35] irqchip: Andestech Internal Vector Interrupt Controller driver
From: Greentime Hu @ 2017-11-29 15:23 UTC (permalink / raw)
To: Marc Zyngier
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Rob Herring, netdev, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Rick Chen
In-Reply-To: <c7447c93-9905-2840-e2d8-01837b9fdecd-5wv7dgnIgG8@public.gmane.org>
Hi, Marc:
2017-11-28 17:37 GMT+08:00 Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>:
> On 27/11/17 12:28, Greentime Hu wrote:
>> +static void ativic32_ack_irq(struct irq_data *data)
>> +{
>> + __nds32__mtsr_dsb(1 << data->hwirq, NDS32_SR_INT_PEND2);
>
> Consider writing (1 << data->hwirq) as BIT(data->hwirq).
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>
> Same here.
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_ack_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb((1 << data->hwirq), NDS32_SR_INT_PEND2);
>
> This is effectively MASK+ACK, so you're better off just writing it as
> such. And since there is no advantage in your implementation in having
> MASK_ACK over MASK+ACK, I suggest you remove this function completely,
> and rely on the core code which will call them in sequence.
I think mask_ack is still better than mask + ack because we don't need
to do two function call.
We can save a prologue and a epilogue. It will benefit interrupt latency.
>> +
>> +}
>> +
>> +static void ativic32_unmask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 | (1 << data->hwirq), NDS32_SR_INT_MASK2);
>
> Same BIT() here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static struct irq_chip ativic32_chip = {
>> + .name = "ativic32",
>> + .irq_ack = ativic32_ack_irq,
>> + .irq_mask = ativic32_mask_irq,
>> + .irq_mask_ack = ativic32_mask_ack_irq,
>> + .irq_unmask = ativic32_unmask_irq,
>> +};
>> +
>> +static unsigned int __initdata nivic_map[6] = { 6, 2, 10, 16, 24, 32 };
>> +
>> +static struct irq_domain *root_domain;
>> +static int ativic32_irq_domain_map(struct irq_domain *id, unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> +
>> + unsigned long int_trigger_type;
>> + int_trigger_type = __nds32__mfsr(NDS32_SR_INT_TRIGGER);
>> + if (int_trigger_type & (1 << hw))
>
> And here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_edge_irq);
>> + else
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_level_irq);
>
> Since you do not express the trigger in DT, you need to tell the core
> about it by calling irqd_set_trigger_type() with the right setting.
>
Since the comments say so, I will add ativic32_set_type() for irq_set_type()
in the next version patch.
/*
* Must only be called inside irq_chip.irq_set_type() functions.
*/
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)
{
__irqd_to_state(d) &= ~IRQD_TRIGGER_MASK;
__irqd_to_state(d) |= type & IRQD_TRIGGER_MASK;
}
It will be like this.
static int ativic32_set_type(struct irq_data *data, unsigned int flow_type)
{
irqd_set_trigger_type(data, flow_type);
return IRQ_SET_MASK_OK;
}
>> +
>> + return 0;
>> +}
>> +
>> +static struct irq_domain_ops ativic32_ops = {
>> + .map = ativic32_irq_domain_map,
>> + .xlate = irq_domain_xlate_onecell
>> +};
>> +
>> +static int get_intr_src(void)
>
> I'm not sure "int" is the best return type here. I suspect something
> unsigned would be preferable, or even the irq_hw_number_t type.
Thanks for this suggestion. I will modify it in the next version patch.
>> +{
>> + return ((__nds32__mfsr(NDS32_SR_ITYPE)&ITYPE_mskVECTOR) >> ITYPE_offVECTOR)
>
> Spaces around '&'.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + - NDS32_VECTOR_offINTERRUPT;
>> +}
>> +
>> +asmlinkage void asm_do_IRQ(struct pt_regs *regs)
>> +{
>> + int hwirq = get_intr_src();
>
> irq_hw_number_t.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + handle_domain_irq(root_domain, hwirq, regs);
>> +}
>> +
>> +int __init ativic32_init_irq(struct device_node *node, struct device_node *parent)
>> +{
>> + unsigned long int_vec_base, nivic;
>> +
>> + if (WARN(parent, "non-root ativic32 are not supported"))
>> + return -EINVAL;
>> +
>> + int_vec_base = __nds32__mfsr(NDS32_SR_IVB);
>> +
>> + if (((int_vec_base & IVB_mskIVIC_VER) >> IVB_offIVIC_VER) == 0)
>> + panic("Unable to use atcivic32 for this cpu.\n");
>> +
>> + nivic = (int_vec_base & IVB_mskNIVIC) >> IVB_offNIVIC;
>> + if (nivic >= (sizeof nivic_map / sizeof nivic_map[0]))
>
> This should be:
> if (nivic >= ARRAY_SIZE(NIVIC_MAP))
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + panic("The number of input for ativic32 is not supported.\n");
>> +
>> + nivic = nivic_map[nivic];
>
> I'd rather you use another variable (nr_ints?).
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +
>> + root_domain = irq_domain_add_linear(node, nivic,
>> + &ativic32_ops, NULL);
>> +
>> + if (!root_domain)
>> + panic("%s: unable to create IRQ domain\n", node->full_name);
>> +
>> + return 0;
>> +}
>> +IRQCHIP_DECLARE(ativic32, "andestech,ativic32", ativic32_init_irq);
>>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox