* Re: [PATCH v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-22 9:53 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, Linus Walleij,
Mark Rutland, Greg KH, Guo Ren, Randy Dunlap <rd>
In-Reply-To: <CAMuHMdUqxuiTfM_m80F4XDVkZjx_XCsj_qXRehZxMcKjWhGQ_Q@mail.gmail.com>
Hi, Geert:
2018-01-19 23:37 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Greentime,
>
> On Fri, Jan 19, 2018 at 4:35 PM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>>>>> From: Greentime Hu <greentime@andestech.com>
>>>>>>>>
>>>>>>>> This patch adds nds32 CPU binding documents.
>>>>>>>>
>>>>>>>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>>>>>>>> Signed-off-by: Rick Chen <rick@andestech.com>
>>>>>>>> Signed-off-by: Zong Li <zong@andestech.com>
>>>>>>>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>> ---
>>>>>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>>>>>> 1 file changed, 37 insertions(+)
>>>>>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..9a52937
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>> @@ -0,0 +1,37 @@
>>>>>>>> +* Andestech Processor Binding
>>>>>>>> +
>>>>>>>> +This binding specifies what properties must be available in the device tree
>>>>>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>>>>>> +tree.
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +
>>>>>>>> + - compatible:
>>>>>>>> + Usage: required
>>>>>>>> + Value type: <string>
>>>>>>>> + Definition: should be one of:
>>>>>>>> + "andestech,n13"
>>>>>>>> + "andestech,n15"
>>>>>>>> + "andestech,d15"
>>>>>>>> + "andestech,n10"
>>>>>>>> + "andestech,d10"
>>>>>>>> + "andestech,nds32v3"
>>>>>>>
>>>>>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>>>>>> the device tree should always list 'andestech,nds32v3' as the most
>>>>>>> generic 'compatible' value and list exactly one of the others in
>>>>>>> addition.
>>>>>
>>>>>> I will remove the others and just left "andestech,nds32v3" in here.
>>>>>
>>>>> No, is not what we want here, the CPU node should list exactly which core
>>>>> is used, what we need in the description is a clarification that
>>>>> andestech,nds32v3 must be used in addition to the more specific
>>>>> string.
>>>>
>>>> Hi, Arnd:
>>>>
>>>> Sorry I still don't get your point. Do you mean we should always use
>>>> compatible = "andestech,n13", "andestech,nds32v3";
>>>> instead of
>>>> compatible = "andestech,n13";
>>>
>>> Exactly. The first value is a device-specific compatible value, the second is
>>> a generic fallback.
>>>
>>>> And I need to add the description in this document.
>>>
>>> Indeed. See for example
>>> Documentation/devicetree/bindings/power/renesas,apmu.txt
>>>
>>> Thanks!
>>
>> Hi, Geert:
>>
>> Thank you and your example.
>> I get it. I will update this document like this.
>> - compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
>> as fallback.
>
> And please keep a list of supported values of "andestech,<core_name>"
> in the DT binding document, so checkpatch can validate compatible values.
>
Thank you for reminding me this.
I will list it like this.
- compatible:
Usage: required
Value type: <string>
Definition: Should be "andestech,<core_name>",
"andestech,nds32v3" as fallback.
Examlpes with core_names are:
"andestech,n13"
"andestech,n15"
"andestech,d15"
"andestech,n10"
"andestech,d10"
^ permalink raw reply
* Re: [PATCH v6 06/36] nds32: Kernel booting and initialization
From: Arnd Bergmann @ 2018-01-22 9:53 UTC (permalink / raw)
To: Greentime Hu
Cc: 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, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAEbi=3dF-ekx8oz0vjd3WfM7qO755rW46Q=+i-MnQC=4N1=EMA@mail.gmail.com>
On Mon, Jan 22, 2018 at 10:49 AM, Greentime Hu <green.hu@gmail.com> wrote:
> 2018-01-20 0:41 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>> This implementation is referenced from openrisc.
>>> https://lkml.org/lkml/2017/11/17/228
>>
>> It's correct on openrisc, because that has a reliable cycle counter,
>> and that gets used in its delay function:
>>
>> void __delay(unsigned long cycles)
>> {
>> cycles_t start = get_cycles();
>> while ((get_cycles() - start) < cycles)
>> cpu_relax();
>> }
>>
>> In my review comment that you cited, I assumed that nds32 had similar
>> hardware.
>>
>> However, as you explained earlier, the nds32 architecture does not provide
>> a cycle counter and the clocksource resolution is not high enough to
>> be a good replacement, so you have to use the traditional delay
>> calibration.
>>
>
> Hi, Arnd:
>
> Thank you for your explanation.
> Will it be ok if I code it like this?
>
> config GENERIC_CALIBRATE_DELAY
> def_bool y
Yes, I think that should be sufficient.
Arnd
^ permalink raw reply
* Re: [PATCH v6 06/36] nds32: Kernel booting and initialization
From: Greentime Hu @ 2018-01-22 9:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: 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, Geert Uytterhoeven,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a3UCrMbTn1JMOGKCSV4WRw9T9tU+pey1t6Fzitf2V0Bvw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-20 0:41 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> On Fri, Jan 19, 2018 at 5:34 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi, Arnd:
>>
>> 2018-01-18 18:11 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> I had not looked at this patch in enough detail earlier, sorry about
>>> that. It should be
>>> easy enough to fix though.
>>>
>>>> +#ifdef CONFIG_VGA_CONSOLE
>>>> +struct screen_info screen_info;
>>>> +#endif
>>>
>>> I would assume that you can't ever have a VGA console. Just drop all
>>> the references
>>> here and instead send a patch to the fbdev maintainer to add the dependency
>>> at CONFIG_VGA_CONSOLE to prevent selecting it with nds32.
>>
>> I found it can be built pass for now because we disable it in defconfig.
>> Should I send the patch in v7 series?
>
> yes, I think that would be best.
>
>>>> +
>>>> +extern void __init early_init_devtree(void *params);
>>>> +extern void __init early_trap_init(void);
>>>
>>> similarly, these are declared in include/linux/of_fdt.h
>>>
>>
>> early_trap_init is a nds32 function. I will move it to nds32.h
>
> Right, makes sense.
>
>>>> +void calibrate_delay(void)
>>>> +{
>>>> + const int *val;
>>>> + struct device_node *cpu = NULL;
>>>> + cpu = of_find_compatible_node(NULL, NULL, "andestech,nds32v3");
>>>> + val = of_get_property(cpu, "clock-frequency", NULL);
>>>> + if (!val || !*val)
>>>> + panic("no cpu 'clock-frequency' parameter in device tree");
>>>> + loops_per_jiffy = be32_to_cpup(val) / HZ;
>>>> + pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
>>>> + loops_per_jiffy / (500000 / HZ),
>>>> + (loops_per_jiffy / (5000 / HZ)) % 100, loops_per_jiffy);
>>>> +}
>>>
>>> This seems very odd to me: The 'clock-frequency' property in the
>>> cpu node should refer to the actual frequency it is running at, but that
>>> tends to be different from the bogomips as needed by the ndelay()
>>> function. Can you explain what is going on here?
>>>
>>
>> This implementation is referenced from openrisc.
>> https://lkml.org/lkml/2017/11/17/228
>
> It's correct on openrisc, because that has a reliable cycle counter,
> and that gets used in its delay function:
>
> void __delay(unsigned long cycles)
> {
> cycles_t start = get_cycles();
> while ((get_cycles() - start) < cycles)
> cpu_relax();
> }
>
> In my review comment that you cited, I assumed that nds32 had similar
> hardware.
>
> However, as you explained earlier, the nds32 architecture does not provide
> a cycle counter and the clocksource resolution is not high enough to
> be a good replacement, so you have to use the traditional delay
> calibration.
>
Hi, Arnd:
Thank you for your explanation.
Will it be ok if I code it like this?
config GENERIC_CALIBRATE_DELAY
def_bool y
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Andy Shevchenko @ 2018-01-22 9:42 UTC (permalink / raw)
To: Hans de Goede, Marcel Holtmann
Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-serial,
linux-acpi, Lukas Wunner
In-Reply-To: <769dd32b-425e-3002-50b5-0d3c707706d4@redhat.com>
On Mon, 2018-01-22 at 09:23 +0100, Hans de Goede wrote:
> Hi,
>
> On 22-01-18 03:24, Marcel Holtmann wrote:
> > Hi Hans,
> >
> > > Now that ACPI and DT devices are both enumerated as serdevs, we
> > > can
> > > remove platform_device support and the bcm_device_list lookup
> > > hack.
> > >
> > > This also removes any races between suspend/resume and hci-uart
> > > binding,
> > > also making the suspend/resume code a lot simpler.
> > >
> > > This commit leaves manually binding to an uart using btattach
> > > supported
> > > (without irq/gpio and thus suspend/resume support, as before).
> > >
> > > Cc: Lukas Wunner <lukas@wunner.de>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > > drivers/bluetooth/hci_bcm.c | 260 +++++---------------------------
> > > ------------
> > > 1 file changed, 28 insertions(+), 232 deletions(-)
> >
> > so I was under the assumption platforms like Intel Edison still only
> > do platform data. See commit
> > 212d71833315c65644efc46223db61dee7b3c68e. Has that changed?
Yes and no.
So, we need that support to satisfy users with classical Edison
firmware.
> Ugh, I was not aware of that and the whole code to match the tty with
> the platform_device on btattach is such a mess and I was actually
> quite
> happy to be able to delete this.
Good idea.
> Andy, I see that you added support for bcm bluetooth over a tty using
> platform_data instead of ACPI enumeration. Can you change the code
> instantiating the device to instead instantiate a serdev, so that we
> kill the platform device support in hci_bcm.c and so that users don't
> need to do a btattach, but instead the kernel will do the attach
> itself
> and things will just work ?
I'm sorry, I can't do this soon, other more priority tasks in a pocket.
The instantiation of the driver is happened in arch/x86/platform/intel-
mid/device_libs/platform_bt.c
I would help with review of any patches till I would able to look at it
myself.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-22 8:23 UTC (permalink / raw)
To: Marcel Holtmann, Andy Shevchenko
Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-serial,
linux-acpi, Lukas Wunner
In-Reply-To: <0EA71635-F2C7-4908-B4DE-F87328A148D0@holtmann.org>
Hi,
On 22-01-18 03:24, Marcel Holtmann wrote:
> Hi Hans,
>
>> Now that ACPI and DT devices are both enumerated as serdevs, we can
>> remove platform_device support and the bcm_device_list lookup hack.
>>
>> This also removes any races between suspend/resume and hci-uart binding,
>> also making the suspend/resume code a lot simpler.
>>
>> This commit leaves manually binding to an uart using btattach supported
>> (without irq/gpio and thus suspend/resume support, as before).
>>
>> Cc: Lukas Wunner <lukas@wunner.de>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/bluetooth/hci_bcm.c | 260 +++++---------------------------------------
>> 1 file changed, 28 insertions(+), 232 deletions(-)
>
> so I was under the assumption platforms like Intel Edison still only do platform data. See commit 212d71833315c65644efc46223db61dee7b3c68e. Has that changed?
Ugh, I was not aware of that and the whole code to match the tty with
the platform_device on btattach is such a mess and I was actually quite
happy to be able to delete this.
Andy, I see that you added support for bcm bluetooth over a tty using
platform_data instead of ACPI enumeration. Can you change the code
instantiating the device to instead instantiate a serdev, so that we
kill the platform device support in hci_bcm.c and so that users don't
need to do a btattach, but instead the kernel will do the attach itself
and things will just work ?
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Marcel Holtmann @ 2018-01-22 2:24 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko
Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-serial,
linux-acpi, Lukas Wunner
In-Reply-To: <20180121214645.15004-1-hdegoede@redhat.com>
Hi Hans,
> Now that ACPI and DT devices are both enumerated as serdevs, we can
> remove platform_device support and the bcm_device_list lookup hack.
>
> This also removes any races between suspend/resume and hci-uart binding,
> also making the suspend/resume code a lot simpler.
>
> This commit leaves manually binding to an uart using btattach supported
> (without irq/gpio and thus suspend/resume support, as before).
>
> Cc: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/bluetooth/hci_bcm.c | 260 +++++---------------------------------------
> 1 file changed, 28 insertions(+), 232 deletions(-)
so I was under the assumption platforms like Intel Edison still only do platform data. See commit 212d71833315c65644efc46223db61dee7b3c68e. Has that changed?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] Bluetooth: hci_bcm: Close serdev on failure to set power on bcm_open()
From: Marcel Holtmann @ 2018-01-22 2:20 UTC (permalink / raw)
To: Hans de Goede
Cc: Gustavo F. Padovan, Johan Hedberg, Bluez mailing list,
linux-serial, ACPI Devel Maling List, Lukas Wunner
In-Reply-To: <20180121214645.15004-2-hdegoede@redhat.com>
Hi Hans,
> Commit 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
> introduced error checking for the bcm_gpio_set_power() call in bcm_open()
> but the error-path it introduces does not properly call
> serdev_device_close() to undo the earlier serdev_device_open(), this
> commit fixes this.
>
> Cc: Lukas Wunner <lukas@wunner.de>
> Fixes: 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/bluetooth/hci_bcm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
can I get this as 1/2 so I can apply it right away.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] acpi, spcr: Make SPCR available to x86
From: Mark Salter @ 2018-01-21 23:21 UTC (permalink / raw)
To: Prarit Bhargava, linux-kernel
Cc: linux-acpi, linux-doc, linux-arm-kernel, linux-pm, linux-serial,
Bhupesh Sharma, Lv Zheng, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Jonathan Corbet, Catalin Marinas,
Will Deacon, Timur Tabi, graeme.gregory, mark.salter
In-Reply-To: <20180118150951.28964-1-prarit@redhat.com>
On Thu, 2018-01-18 at 10:09 -0500, Prarit Bhargava wrote:
> Note on testing: I've tested this on several ARM64 and x86 boxes (only
> earlycon, console=ttyS0,115200, and with both options), verified that
> functionality on ARM64 has not changed (ie, CONFIG_ACPI_SPCR_TABLE is
> always =y), and verified functionality when !CONFIG_ACPI_SPCR_TABLE on
> x86.
>
> P.
>
> ----8<----
>
> SPCR is currently only enabled or ARM64 and x86 can use SPCR to setup an
> early console.
>
> General fixes include updating Documentation & Kconfig (for x86), updating
> comments, and changing parse_spcr() to acpi_parse_spcr(), and
> earlycon_init_is_deferred to earlycon_acpi_spcr_enable to be more
> descriptive.
>
> On x86, many systems have a valid SPCR table but the table version is not
> 2 so the table version check must be a warning.
>
> On ARM64 when the kernel parameter earlycon is used both the early console
> and console are enabled. On x86, only the earlycon should be enabled by
> by default. Modify acpi_parse_spcr() to allow options for initializing
> the early console and console separately.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" Mrjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> Cc: graeme.gregory@linaro.org
> Cc: mark.salter@redhat.com
> ---
> Documentation/admin-guide/kernel-parameters.txt | 9 +++++---
> arch/arm64/kernel/acpi.c | 4 ++--
> arch/x86/kernel/acpi/boot.c | 3 +++
> drivers/acpi/Kconfig | 7 +++++-
> drivers/acpi/spcr.c | 29 +++++++++++++------------
> drivers/tty/serial/earlycon.c | 15 +++++--------
> include/linux/acpi.h | 7 ++++--
> include/linux/serial_core.h | 4 ++--
> 8 files changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 46b26bfee27b..6f519230ed34 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -915,9 +915,12 @@
>
> earlycon= [KNL] Output early console device and options.
>
> - When used with no options, the early console is
> - determined by the stdout-path property in device
> - tree's chosen node.
> + [ARM64] The early console is determined by the
> + stdout-path property in device tree's chosen node,
> + or determined by the ACPI SPCR table.
> +
> + [X86] When used with no options the early console is
> + determined by the ACPI SPCR table.
>
> cdns,<addr>[,options]
> Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..2aa5609def27 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -230,10 +230,10 @@ void __init acpi_boot_table_init(void)
>
> done:
> if (acpi_disabled) {
> - if (earlycon_init_is_deferred)
> + if (earlycon_acpi_spcr_enable)
> early_init_dt_scan_chosen_stdout();
> } else {
> - parse_spcr(earlycon_init_is_deferred);
> + acpi_parse_spcr(earlycon_acpi_spcr_enable, true);
> if (IS_ENABLED(CONFIG_ACPI_BGRT))
> acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
> }
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index f4c463df8b08..0bf6a58f7a6d 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -36,6 +36,7 @@
> #include <linux/ioport.h>
> #include <linux/pci.h>
> #include <linux/efi-bgrt.h>
> +#include <linux/serial_core.h>
>
> #include <asm/e820/api.h>
> #include <asm/irqdomain.h>
> @@ -1626,6 +1627,8 @@ int __init acpi_boot_init(void)
> if (!acpi_noirq)
> x86_init.pci.init = pci_acpi_init;
>
> + /* Do not enable ACPI SPCR console by default */
> + acpi_parse_spcr(earlycon_acpi_spcr_enable, false);
> return 0;
> }
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..ddcb5f40e8ee 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
> endif
>
> config ACPI_SPCR_TABLE
> - bool
> + bool "ACPI Serial Port Console Redirection Support"
> + default y if X86
> + help
> + Enable support for Serial Port Console Redirection (SPCR) Table.
> + This table provides information about the configuration of the
> + earlycon console.
>
> config ACPI_LPIT
> bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..89e97d21a89c 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -21,7 +21,7 @@
> * occasionally getting stuck as 1. To avoid the potential for a hang, check
> * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> + * acpi_parse_spcr().
> */
> bool qdf2400_e44_present;
> EXPORT_SYMBOL(qdf2400_e44_present);
> @@ -74,19 +74,21 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> }
>
> /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
> *
> - * @earlycon: set up earlycon for the console specified by the table
> + * @enable_earlycon: set up earlycon for the console specified by the table
> + * @enable_console: setup the console specified by the table.
> *
> * For the architectures with support for ACPI, CONFIG_ACPI_SPCR_TABLE may be
> * defined to parse ACPI SPCR table. As a result of the parsing preferred
> - * console is registered and if @earlycon is true, earlycon is set up.
> + * console is registered and if @enable_earlycon is true, earlycon is set up.
> + * If @enable_console is true the system console is also configured.
> *
> * When CONFIG_ACPI_SPCR_TABLE is defined, this function should be called
> * from arch initialization code as soon as the DT/ACPI decision is made.
> *
> */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
> {
> static char opts[64];
> struct acpi_table_spcr *table;
> @@ -105,11 +107,8 @@ int __init parse_spcr(bool earlycon)
> if (ACPI_FAILURE(status))
> return -ENOENT;
>
> - if (table->header.revision < 2) {
> - err = -ENOENT;
> - pr_err("wrong table version\n");
> - goto done;
> - }
> + if (table->header.revision < 2)
> + pr_info("SPCR table version %d\n", table->header.revision);
>
> if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> switch (ACPI_ACCESS_BIT_WIDTH((
> @@ -185,7 +184,7 @@ int __init parse_spcr(bool earlycon)
> */
> if (qdf2400_erratum_44_present(&table->header)) {
> qdf2400_e44_present = true;
> - if (earlycon)
> + if (enable_earlycon)
> uart = "qdf2400_e44";
> }
>
> @@ -205,11 +204,13 @@ int __init parse_spcr(bool earlycon)
>
> pr_info("console: %s\n", opts);
>
> - if (earlycon)
> + if (enable_earlycon)
> setup_earlycon(opts);
>
> - err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -
> + if (enable_console)
> + err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> + else
> + err = 0;
> done:
> acpi_put_table((struct acpi_table_header *)table);
> return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..870e84fb6e39 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -197,25 +197,20 @@ int __init setup_earlycon(char *buf)
> }
>
> /*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made. At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> + * This defers the initialization of the early console until after ACPI has
> + * been initialized.
> */
> -bool earlycon_init_is_deferred __initdata;
> +bool earlycon_acpi_spcr_enable __initdata;
>
> /* early_param wrapper for setup_earlycon() */
> static int __init param_setup_earlycon(char *buf)
> {
> int err;
>
> - /*
> - * Just 'earlycon' is a valid param for devicetree earlycons;
> - * don't generate a warning from parse_early_params() in that case
> - */
> + /* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
> if (!buf || !buf[0]) {
> if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> - earlycon_init_is_deferred = true;
> + earlycon_acpi_spcr_enable = true;
> return 0;
> } else if (!buf) {
> return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..9aac8f0ebe23 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1242,9 +1242,12 @@ static inline void acpi_table_upgrade(void) { }
>
> #ifdef CONFIG_ACPI_SPCR_TABLE
> extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +int acpi_parse_spcr(bool enable_earlycon, bool enable_console);
> #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static inline int acpi_parse_spcr(bool enable_earlycon, bool enable_console)
> +{
> + return 0;
> +}
> #endif
>
> #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 37b044e78333..aefd0e5115da 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -376,10 +376,10 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
> const char *options);
>
> #ifdef CONFIG_SERIAL_EARLYCON
> -extern bool earlycon_init_is_deferred __initdata;
> +extern bool earlycon_acpi_spcr_enable __initdata;
> int setup_earlycon(char *buf);
> #else
> -static const bool earlycon_init_is_deferred;
> +static const bool earlycon_acpi_spcr_enable;
> static inline int setup_earlycon(char *buf) { return 0; }
> #endif
>
This looks okay to me. Tested on a variety of arm64 platforms.
Reviewed-by: Mark Salter <msalter@redhat.com>
Tested-by: Mark Salter <msalter@redhat.com>
^ permalink raw reply
* [PATCH 2/2] Bluetooth: hci_bcm: Close serdev on failure to set power on bcm_open()
From: Hans de Goede @ 2018-01-21 21:46 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Hans de Goede, linux-bluetooth, linux-serial, linux-acpi,
Lukas Wunner
In-Reply-To: <20180121214645.15004-1-hdegoede@redhat.com>
Commit 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
introduced error checking for the bcm_gpio_set_power() call in bcm_open()
but the error-path it introduces does not properly call
serdev_device_close() to undo the earlier serdev_device_open(), this
commit fixes this.
Cc: Lukas Wunner <lukas@wunner.de>
Fixes: 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/bluetooth/hci_bcm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 61f73cc4c05e..da4736f2e913 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -336,11 +336,13 @@ static int bcm_open(struct hci_uart *hu)
hu->oper_speed = bdev->oper_speed;
err = bcm_gpio_set_power(bdev, true);
if (err)
- goto err_free;
+ goto err_close_serdev;
}
return 0;
+err_close_serdev:
+ serdev_device_close(hu->serdev);
err_free:
hu->priv = NULL;
kfree(bcm);
--
2.14.3
^ permalink raw reply related
* [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-21 21:46 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Hans de Goede, linux-bluetooth, linux-serial, linux-acpi,
Lukas Wunner
Now that ACPI and DT devices are both enumerated as serdevs, we can
remove platform_device support and the bcm_device_list lookup hack.
This also removes any races between suspend/resume and hci-uart binding,
also making the suspend/resume code a lot simpler.
This commit leaves manually binding to an uart using btattach supported
(without irq/gpio and thus suspend/resume support, as before).
Cc: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/bluetooth/hci_bcm.c | 260 +++++---------------------------------------
1 file changed, 28 insertions(+), 232 deletions(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 64800cd2796c..61f73cc4c05e 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -30,7 +30,6 @@
#include <linux/of.h>
#include <linux/property.h>
#include <linux/platform_data/x86/apple.h>
-#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
#include <linux/tty.h>
@@ -80,9 +79,6 @@
* set to 0 if @init_speed is already the preferred baudrate
* @irq: interrupt triggered by HOST_WAKE_BT pin
* @irq_active_low: whether @irq is active low
- * @hu: pointer to HCI UART controller struct,
- * used to disable flow control during runtime suspend and system sleep
- * @is_suspended: whether flow control is currently disabled
*/
struct bcm_device {
/* Must be the first member, hci_serdev.c expects this. */
@@ -107,25 +103,14 @@ struct bcm_device {
u32 oper_speed;
int irq;
bool irq_active_low;
-
-#ifdef CONFIG_PM
- struct hci_uart *hu;
- bool is_suspended;
-#endif
};
/* generic bcm uart resources */
struct bcm_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
-
- struct bcm_device *dev;
};
-/* List of BCM BT UART devices */
-static DEFINE_MUTEX(bcm_device_lock);
-static LIST_HEAD(bcm_device_list);
-
static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
{
if (hu->serdev)
@@ -183,27 +168,6 @@ static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
return 0;
}
-/* bcm_device_exists should be protected by bcm_device_lock */
-static bool bcm_device_exists(struct bcm_device *device)
-{
- struct list_head *p;
-
-#ifdef CONFIG_PM
- /* Devices using serdev always exist */
- if (device && device->hu && device->hu->serdev)
- return true;
-#endif
-
- list_for_each(p, &bcm_device_list) {
- struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
- if (device == dev)
- return true;
- }
-
- return false;
-}
-
static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
{
int err;
@@ -249,21 +213,17 @@ static irqreturn_t bcm_host_wake(int irq, void *data)
return IRQ_HANDLED;
}
-static int bcm_request_irq(struct bcm_data *bcm)
+static int bcm_request_irq(struct hci_uart *hu)
{
- struct bcm_device *bdev = bcm->dev;
+ struct bcm_device *bdev;
int err;
- mutex_lock(&bcm_device_lock);
- if (!bcm_device_exists(bdev)) {
- err = -ENODEV;
- goto unlock;
- }
+ if (!hu->serdev)
+ return -ENODEV;
- if (bdev->irq <= 0) {
- err = -EOPNOTSUPP;
- goto unlock;
- }
+ bdev = serdev_device_get_drvdata(hu->serdev);
+ if (bdev->irq <= 0)
+ return -EOPNOTSUPP;
err = devm_request_irq(bdev->dev, bdev->irq, bcm_host_wake,
bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
@@ -271,7 +231,7 @@ static int bcm_request_irq(struct bcm_data *bcm)
"host_wake", bdev);
if (err) {
bdev->irq = err;
- goto unlock;
+ return err;
}
device_init_wakeup(bdev->dev, true);
@@ -282,10 +242,7 @@ static int bcm_request_irq(struct bcm_data *bcm)
pm_runtime_set_active(bdev->dev);
pm_runtime_enable(bdev->dev);
-unlock:
- mutex_unlock(&bcm_device_lock);
-
- return err;
+ return 0;
}
static const struct bcm_set_sleep_mode default_sleep_params = {
@@ -306,11 +263,11 @@ static const struct bcm_set_sleep_mode default_sleep_params = {
static int bcm_setup_sleep(struct hci_uart *hu)
{
- struct bcm_data *bcm = hu->priv;
+ struct bcm_device *bdev = serdev_device_get_drvdata(hu->serdev);
struct sk_buff *skb;
struct bcm_set_sleep_mode sleep_params = default_sleep_params;
- sleep_params.host_wake_active = !bcm->dev->irq_active_low;
+ sleep_params.host_wake_active = !bdev->irq_active_low;
skb = __hci_cmd_sync(hu->hdev, 0xfc27, sizeof(sleep_params),
&sleep_params, HCI_INIT_TIMEOUT);
@@ -326,7 +283,7 @@ static int bcm_setup_sleep(struct hci_uart *hu)
return 0;
}
#else
-static inline int bcm_request_irq(struct bcm_data *bcm) { return 0; }
+static inline int bcm_request_irq(struct hci_uart *hu) { return 0; }
static inline int bcm_setup_sleep(struct hci_uart *hu) { return 0; }
#endif
@@ -356,7 +313,6 @@ static int bcm_set_diag(struct hci_dev *hdev, bool enable)
static int bcm_open(struct hci_uart *hu)
{
struct bcm_data *bcm;
- struct list_head *p;
int err;
bt_dev_dbg(hu->hdev, "hu %p", hu);
@@ -369,54 +325,23 @@ static int bcm_open(struct hci_uart *hu)
hu->priv = bcm;
- mutex_lock(&bcm_device_lock);
-
if (hu->serdev) {
+ struct bcm_device *bdev = serdev_device_get_drvdata(hu->serdev);
+
err = serdev_device_open(hu->serdev);
if (err)
goto err_free;
- bcm->dev = serdev_device_get_drvdata(hu->serdev);
- goto out;
- }
-
- if (!hu->tty->dev)
- goto out;
-
- list_for_each(p, &bcm_device_list) {
- struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
- /* Retrieve saved bcm_device based on parent of the
- * platform device (saved during device probe) and
- * parent of tty device used by hci_uart
- */
- if (hu->tty->dev->parent == dev->dev->parent) {
- bcm->dev = dev;
-#ifdef CONFIG_PM
- dev->hu = hu;
-#endif
- break;
- }
- }
-
-out:
- if (bcm->dev) {
- hu->init_speed = bcm->dev->init_speed;
- hu->oper_speed = bcm->dev->oper_speed;
- err = bcm_gpio_set_power(bcm->dev, true);
+ hu->init_speed = bdev->init_speed;
+ hu->oper_speed = bdev->oper_speed;
+ err = bcm_gpio_set_power(bdev, true);
if (err)
- goto err_unset_hu;
+ goto err_free;
}
- mutex_unlock(&bcm_device_lock);
return 0;
-err_unset_hu:
-#ifdef CONFIG_PM
- bcm->dev->hu = NULL;
-#endif
err_free:
- mutex_unlock(&bcm_device_lock);
hu->priv = NULL;
kfree(bcm);
return err;
@@ -430,20 +355,10 @@ static int bcm_close(struct hci_uart *hu)
bt_dev_dbg(hu->hdev, "hu %p", hu);
- /* Protect bcm->dev against removal of the device or driver */
- mutex_lock(&bcm_device_lock);
-
if (hu->serdev) {
serdev_device_close(hu->serdev);
bdev = serdev_device_get_drvdata(hu->serdev);
- } else if (bcm_device_exists(bcm->dev)) {
- bdev = bcm->dev;
-#ifdef CONFIG_PM
- bdev->hu = NULL;
-#endif
- }
- if (bdev) {
if (IS_ENABLED(CONFIG_PM) && bdev->irq > 0) {
devm_free_irq(bdev->dev, bdev->irq, bdev);
device_init_wakeup(bdev->dev, false);
@@ -456,7 +371,6 @@ static int bcm_close(struct hci_uart *hu)
else
pm_runtime_set_suspended(bdev->dev);
}
- mutex_unlock(&bcm_device_lock);
skb_queue_purge(&bcm->txq);
kfree_skb(bcm->rx_skb);
@@ -479,7 +393,6 @@ static int bcm_flush(struct hci_uart *hu)
static int bcm_setup(struct hci_uart *hu)
{
- struct bcm_data *bcm = hu->priv;
char fw_name[64];
const struct firmware *fw;
unsigned int speed;
@@ -538,7 +451,7 @@ static int bcm_setup(struct hci_uart *hu)
if (err)
return err;
- if (!bcm_request_irq(bcm))
+ if (!bcm_request_irq(hu))
err = bcm_setup_sleep(hu);
return err;
@@ -580,12 +493,9 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
bcm->rx_skb = NULL;
return err;
- } else if (!bcm->rx_skb) {
+ } else if (!bcm->rx_skb && hu->serdev) {
/* Delay auto-suspend when receiving completed packet */
- mutex_lock(&bcm_device_lock);
- if (bcm->dev && bcm_device_exists(bcm->dev))
- pm_request_resume(bcm->dev->dev);
- mutex_unlock(&bcm_device_lock);
+ pm_request_resume(&hu->serdev->dev);
}
return count;
@@ -610,10 +520,8 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
struct sk_buff *skb = NULL;
struct bcm_device *bdev = NULL;
- mutex_lock(&bcm_device_lock);
-
- if (bcm_device_exists(bcm->dev)) {
- bdev = bcm->dev;
+ if (hu->serdev) {
+ bdev = serdev_device_get_drvdata(hu->serdev);
pm_runtime_get_sync(bdev->dev);
/* Shall be resumed here */
}
@@ -625,8 +533,6 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
pm_runtime_put_autosuspend(bdev->dev);
}
- mutex_unlock(&bcm_device_lock);
-
return skb;
}
@@ -638,20 +544,12 @@ static int bcm_suspend_device(struct device *dev)
bt_dev_dbg(bdev, "");
- if (!bdev->is_suspended && bdev->hu) {
- hci_uart_set_flow_control(bdev->hu, true);
-
- /* Once this returns, driver suspends BT via GPIO */
- bdev->is_suspended = true;
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, true);
/* Suspend the device */
err = bdev->set_device_wakeup(bdev, false);
if (err) {
- if (bdev->is_suspended && bdev->hu) {
- bdev->is_suspended = false;
- hci_uart_set_flow_control(bdev->hu, false);
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, false);
return -EBUSY;
}
@@ -678,11 +576,7 @@ static int bcm_resume_device(struct device *dev)
msleep(15);
/* When this executes, the device has woken up already */
- if (bdev->is_suspended && bdev->hu) {
- bdev->is_suspended = false;
-
- hci_uart_set_flow_control(bdev->hu, false);
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, false);
return 0;
}
@@ -695,18 +589,7 @@ static int bcm_suspend(struct device *dev)
struct bcm_device *bdev = dev_get_drvdata(dev);
int error;
- bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
-
- /*
- * When used with a device instantiated as platform_device, bcm_suspend
- * can be called at any time as long as the platform device is bound,
- * so it should use bcm_device_lock to protect access to hci_uart
- * and device_wake-up GPIO.
- */
- mutex_lock(&bcm_device_lock);
-
- if (!bdev->hu)
- goto unlock;
+ bt_dev_dbg(bdev, "");
if (pm_runtime_active(dev))
bcm_suspend_device(dev);
@@ -717,9 +600,6 @@ static int bcm_suspend(struct device *dev)
bt_dev_dbg(bdev, "BCM irq: enabled");
}
-unlock:
- mutex_unlock(&bcm_device_lock);
-
return 0;
}
@@ -729,18 +609,7 @@ static int bcm_resume(struct device *dev)
struct bcm_device *bdev = dev_get_drvdata(dev);
int err = 0;
- bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
-
- /*
- * When used with a device instantiated as platform_device, bcm_resume
- * can be called at any time as long as platform device is bound,
- * so it should use bcm_device_lock to protect access to hci_uart
- * and device_wake-up GPIO.
- */
- mutex_lock(&bcm_device_lock);
-
- if (!bdev->hu)
- goto unlock;
+ bt_dev_dbg(bdev, "");
if (device_may_wakeup(dev) && bdev->irq > 0) {
disable_irq_wake(bdev->irq);
@@ -748,10 +617,6 @@ static int bcm_resume(struct device *dev)
}
err = bcm_resume_device(dev);
-
-unlock:
- mutex_unlock(&bcm_device_lock);
-
if (!err) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
@@ -1002,57 +867,6 @@ static int bcm_of_probe(struct bcm_device *bdev)
return 0;
}
-static int bcm_probe(struct platform_device *pdev)
-{
- struct bcm_device *dev;
- int ret;
-
- dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
- return -ENOMEM;
-
- dev->dev = &pdev->dev;
- dev->irq = platform_get_irq(pdev, 0);
-
- if (has_acpi_companion(&pdev->dev)) {
- ret = bcm_acpi_probe(dev);
- if (ret)
- return ret;
- }
-
- ret = bcm_get_resources(dev);
- if (ret)
- return ret;
-
- platform_set_drvdata(pdev, dev);
-
- dev_info(&pdev->dev, "%s device registered.\n", dev->name);
-
- /* Place this instance on the device list */
- mutex_lock(&bcm_device_lock);
- list_add_tail(&dev->list, &bcm_device_list);
- mutex_unlock(&bcm_device_lock);
-
- ret = bcm_gpio_set_power(dev, false);
- if (ret)
- dev_err(&pdev->dev, "Failed to power down\n");
-
- return 0;
-}
-
-static int bcm_remove(struct platform_device *pdev)
-{
- struct bcm_device *dev = platform_get_drvdata(pdev);
-
- mutex_lock(&bcm_device_lock);
- list_del(&dev->list);
- mutex_unlock(&bcm_device_lock);
-
- dev_info(&pdev->dev, "%s device unregistered.\n", dev->name);
-
- return 0;
-}
-
static const struct hci_uart_proto bcm_proto = {
.id = HCI_UART_BCM,
.name = "Broadcom",
@@ -1100,16 +914,6 @@ static const struct dev_pm_ops bcm_pm_ops = {
SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
};
-static struct platform_driver bcm_driver = {
- .probe = bcm_probe,
- .remove = bcm_remove,
- .driver = {
- .name = "hci_bcm",
- .acpi_match_table = ACPI_PTR(bcm_acpi_match),
- .pm = &bcm_pm_ops,
- },
-};
-
static int bcm_serdev_probe(struct serdev_device *serdev)
{
struct bcm_device *bcmdev;
@@ -1120,9 +924,6 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;
bcmdev->dev = &serdev->dev;
-#ifdef CONFIG_PM
- bcmdev->hu = &bcmdev->serdev_hu;
-#endif
bcmdev->serdev_hu.serdev = serdev;
serdev_device_set_drvdata(serdev, bcmdev);
@@ -1172,10 +973,6 @@ static struct serdev_device_driver bcm_serdev_driver = {
int __init bcm_init(void)
{
- /* For now, we need to keep both platform device
- * driver (ACPI generated) and serdev driver (DT).
- */
- platform_driver_register(&bcm_driver);
serdev_device_driver_register(&bcm_serdev_driver);
return hci_uart_register_proto(&bcm_proto);
@@ -1183,7 +980,6 @@ int __init bcm_init(void)
int __exit bcm_deinit(void)
{
- platform_driver_unregister(&bcm_driver);
serdev_device_driver_unregister(&bcm_serdev_driver);
return hci_uart_unregister_proto(&bcm_proto);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH] acpi, spcr: Make SPCR available to x86
From: Ingo Molnar @ 2018-01-20 14:33 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel, linux-acpi, linux-doc, linux-arm-kernel, linux-pm,
linux-serial, Bhupesh Sharma, Lv Zheng, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86, Jonathan Corbet,
Catalin Marinas, Will Deacon, Timur Tabi, graeme.gregory,
mark.salter
In-Reply-To: <20180118150951.28964-1-prarit@redhat.com>
* Prarit Bhargava <prarit@redhat.com> wrote:
> Note on testing: I've tested this on several ARM64 and x86 boxes (only
> earlycon, console=ttyS0,115200, and with both options), verified that
> functionality on ARM64 has not changed (ie, CONFIG_ACPI_SPCR_TABLE is
> always =y), and verified functionality when !CONFIG_ACPI_SPCR_TABLE on
> x86.
>
> P.
>
> ----8<----
>
> SPCR is currently only enabled or ARM64 and x86 can use SPCR to setup an
> early console.
>
> General fixes include updating Documentation & Kconfig (for x86), updating
> comments, and changing parse_spcr() to acpi_parse_spcr(), and
> earlycon_init_is_deferred to earlycon_acpi_spcr_enable to be more
> descriptive.
>
> On x86, many systems have a valid SPCR table but the table version is not
> 2 so the table version check must be a warning.
>
> On ARM64 when the kernel parameter earlycon is used both the early console
> and console are enabled. On x86, only the earlycon should be enabled by
> by default. Modify acpi_parse_spcr() to allow options for initializing
> the early console and console separately.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" Mrjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> Cc: graeme.gregory@linaro.org
> Cc: mark.salter@redhat.com
> ---
> Documentation/admin-guide/kernel-parameters.txt | 9 +++++---
> arch/arm64/kernel/acpi.c | 4 ++--
> arch/x86/kernel/acpi/boot.c | 3 +++
> drivers/acpi/Kconfig | 7 +++++-
> drivers/acpi/spcr.c | 29 +++++++++++++------------
> drivers/tty/serial/earlycon.c | 15 +++++--------
> include/linux/acpi.h | 7 ++++--
> include/linux/serial_core.h | 4 ++--
> 8 files changed, 44 insertions(+), 34 deletions(-)
I'm fine with the x86 aspect of this:
Acked-by: Ingo Molnar <mingo@kernel.org>
> earlycon= [KNL] Output early console device and options.
>
> - When used with no options, the early console is
> - determined by the stdout-path property in device
> - tree's chosen node.
> + [ARM64] The early console is determined by the
> + stdout-path property in device tree's chosen node,
> + or determined by the ACPI SPCR table.
s/in device tree's chosen node
/in the device tree's chosen node
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v6 2/3] clocksource/drivers/atcpit100: VDSO support
From: Vincent Chen @ 2018-01-20 11:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greentime Hu, Greentime, Linux Kernel Mailing List, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Networking, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAJsyPhxrKKN43usZ+mxAzci4G1qXnEXQ6ydT18MCiT7qQcMz3g@mail.gmail.com>
Correct the composition
Dear Arnd:
1. Setup an additional memory mapping for clock hardware in user space
when establishing vdso-needed memory mapping
In arch_setup_additional_pages(), kernel establishes memory
mapping for vdso's text and vdata page in user space. In order to make
clock hardware be accessible in user space, we try to establish an
additional memory mapping for clock hardware here based on clock
information from driver. This page is located between vdata page and
vdso text page. For safety, this region for clock accessing is
read-only.
2. Accessing clock hardware in vdso
After step 1, clock hardware is accessible in user space through
memory-mapped IO. However, it is not enough to access a specific
register. Therefore, we store register offset information in vdata
page to make it visible in user space. Now, vdso can derive the
address of counter register by summation of __get_timerpage() and
counter register offset where __get_timerpage() is used to derive the
virtual address of memory-mapped clock.
2018-01-20 19:11 GMT+08:00 Vincent Chen <deanbo422@gmail.com>:
> 2018-01-18 19:08 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>> On Mon, Jan 15, 2018 at 6:57 AM, Greentime Hu <green.hu@gmail.com> wrote:
>>> From: Rick Chen <rickchen36@gmail.com>
>>>
>>> VDSO needs real-time cycle count to ensure the time accuracy.
>>> Unlike others, nds32 architecture does not define clock source,
>>> hence VDSO needs atcpit100 offering real-time cycle count
>>> to derive the correct time.
>>>
>>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>>> Signed-off-by: Rick Chen <rickchen36@gmail.com>
>>> Signed-off-by: Greentime Hu <green.hu@gmail.com>
>>
>> I'm a bit puzzled by this patch, can you explain how the vdso actually
>> manages to access the clock hardware? It looks like you make the
>> physical address and the register offset available to user space, but
>> how does it end up accessing it?
>>
>> Arnd
>
> Dear Arnd:
>
> Accessing clock hardware in vdso can be divided to 2 step.
>
> 1. Setup an additional memory mapping for clock hardware in user space
> when establishing
> vdso-needed memory mapping
>
> In arch_setup_additional_pages(), kernel establishes memory
> mapping for vdso's text and vdata page
> in user space. In order to make clock hardware be accessible in
> user space, we try to establish an
> additional memory mapping for clock hardware here based on clock
> information from driver. This page is
> located between vdata page and vdso text page. For safety, this
> region for clock accessing is read-only.
>
> 2. Accessing clock hardware in vdso
>
> After step 1, clock hardware is accessible in user space
> through memory-mapped IO. However, it is not
> enough to access a specific register. Therefore, we store register
> offset information in vdata page to make it
> visible in user space. Now, vdso can derive the address of counter
> register by summation of __get_timerpage()
> and counter register offset where __get_timerpage() is used to
> derive the virtual address of memory-mapped
> clock.
>
>
> Vincent
^ permalink raw reply
* Re: [PATCH v6 2/3] clocksource/drivers/atcpit100: VDSO support
From: Vincent Chen @ 2018-01-20 11:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greentime Hu, Greentime, Linux Kernel Mailing List, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Networking, DTML, Al Viro, David Howells, Will Deacon,
Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a1RQxBNGiVD11BuL5kUx+fn0QcA02j0HVPnMQ2j3WmMQQ@mail.gmail.com>
2018-01-18 19:08 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Jan 15, 2018 at 6:57 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> From: Rick Chen <rickchen36@gmail.com>
>>
>> VDSO needs real-time cycle count to ensure the time accuracy.
>> Unlike others, nds32 architecture does not define clock source,
>> hence VDSO needs atcpit100 offering real-time cycle count
>> to derive the correct time.
>>
>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>> Signed-off-by: Rick Chen <rickchen36@gmail.com>
>> Signed-off-by: Greentime Hu <green.hu@gmail.com>
>
> I'm a bit puzzled by this patch, can you explain how the vdso actually
> manages to access the clock hardware? It looks like you make the
> physical address and the register offset available to user space, but
> how does it end up accessing it?
>
> Arnd
Dear Arnd:
Accessing clock hardware in vdso can be divided to 2 step.
1. Setup an additional memory mapping for clock hardware in user space
when establishing
vdso-needed memory mapping
In arch_setup_additional_pages(), kernel establishes memory
mapping for vdso's text and vdata page
in user space. In order to make clock hardware be accessible in
user space, we try to establish an
additional memory mapping for clock hardware here based on clock
information from driver. This page is
located between vdata page and vdso text page. For safety, this
region for clock accessing is read-only.
2. Accessing clock hardware in vdso
After step 1, clock hardware is accessible in user space
through memory-mapped IO. However, it is not
enough to access a specific register. Therefore, we store register
offset information in vdata page to make it
visible in user space. Now, vdso can derive the address of counter
register by summation of __get_timerpage()
and counter register offset where __get_timerpage() is used to
derive the virtual address of memory-mapped
clock.
Vincent
^ permalink raw reply
* Re: [PATCH v2 3/7] soc: qcom: Add GENI based QUP Wrapper driver
From: Rob Herring @ 2018-01-19 22:57 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rajendra Nayak, Karthikeyan Ramasubramanian, corbet, andy.gross,
david.brown, mark.rutland, wsa, gregkh, linux-doc, linux-arm-msm,
devicetree, linux-i2c, linux-serial, jslaby, Sagar Dharia,
Girish Mahadevan
In-Reply-To: <20180118165745.GE6620@minitux>
On Thu, Jan 18, 2018 at 08:57:45AM -0800, Bjorn Andersson wrote:
> On Thu 18 Jan 01:13 PST 2018, Rajendra Nayak wrote:
>
> > []..
> >
> > >> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> > >> new file mode 100644
> > >> index 0000000..3f43582
> > >> --- /dev/null
> > >> +++ b/drivers/soc/qcom/qcom-geni-se.c
> > >> @@ -0,0 +1,1016 @@
> > >> +/*
> > >> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> > >> + *
> > >> + * This program is free software; you can redistribute it and/or modify
> > >> + * it under the terms of the GNU General Public License version 2 and
> > >> + * only version 2 as published by the Free Software Foundation.
> > >> + *
> > >> + * This program is distributed in the hope that it will be useful,
> > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > >> + * GNU General Public License for more details.
> > >> + *
> > >> + */
> > >
> > > Please use SPDX style license header, i.e. this file should start with:
> > >
> > > // SPDX-License-Identifier: GPL-2.0
> > > /*
> > > * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> > > */
> >
> > Looks like Mark Brown commented elsewhere [1] that we should use the C++
> > commenting style even for the Linux Foundation copyright?
> >
> > [1] https://marc.info/?l=linux-clk&m=151497978626135&w=2
> >
>
> While I can agree with Mark on the ugliness of the mixed commenting
> style, this is the style that I found communicated and is what you find
> in other files.
Well, that's pretty new guidance. Moving target...
Given that Linus said '//' comments are the only thing C++ got right, I
expect to see more of them.
Rob
^ permalink raw reply
* Re: [PATCH v2 2/7] dt-bindings: soc: qcom: Add device tree binding for GENI SE
From: Rob Herring @ 2018-01-19 22:53 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Karthikeyan Ramasubramanian, corbet, andy.gross, david.brown,
mark.rutland, wsa, gregkh, linux-doc, linux-arm-msm, devicetree,
linux-i2c, linux-serial, jslaby
In-Reply-To: <20180117062558.GB6620@minitux>
On Tue, Jan 16, 2018 at 10:25:58PM -0800, Bjorn Andersson wrote:
> On Fri 12 Jan 17:05 PST 2018, Karthikeyan Ramasubramanian wrote:
>
> > Add device tree binding support for the QCOM GENI SE driver.
> >
> > Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
>
> Better describe the entire GENI, with it's functions in the same
> binding.
>
> > ---
> > .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 34 ++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> > new file mode 100644
> > index 0000000..66f8a16
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> > @@ -0,0 +1,34 @@
> > +Qualcomm Technologies, Inc. GENI Serial Engine QUP Wrapper Controller
> > +
> > +Generic Interface (GENI) based Qualcomm Universal Peripheral (QUP) wrapper
> > +is a programmable module for supporting a wide range of serial interfaces
> > +like UART, SPI, I2C, I3C, etc. A single QUP module can provide upto 8 Serial
> > +Interfaces, using its internal Serial Engines. The GENI Serial Engine QUP
> > +Wrapper controller is modeled as a node with zero or more child nodes each
> > +representing a serial engine.
> > +
> > +Required properties:
> > +- compatible: Must be "qcom,geni-se-qup".
> > +- reg: Must contain QUP register address and length.
> > +- clock-names: Must contain "m-ahb" and "s-ahb".
> > +- clocks: AHB clocks needed by the device.
> > +
> > +Required properties if child node exists:
> > +- #address-cells: Must be 1
> > +- #size-cells: Must be 1
>
> But on a 64-bit system, shouldn't these be 2?
No, depends on the bus and ranges. It annoys me to no end that folks
needlessly use 2 cells when the perpheral address space and/or module
sizes don't exceed 4G.
>
> > +- ranges: Must be present
And ideally with a value, but that's beyond the scope of the binding.
> > +
> > +Properties for children:
> > +
> > +A GENI based QUP wrapper controller node can contain 0 or more child nodes
> > +representing serial devices. These serial devices can be a QCOM UART, I2C
> > +controller, spi controller, or some combination of aforementioned devices.
> > +
> > +Example:
> > + qup0: qcom,geniqup0@8c0000 {
>
> I don't see a reason for this to be referenced, so skip the label. And
> don't use qcom, in the node name; "geni" would be perfectly fine?
>
> > + compatible = "qcom,geni-se-qup";
> > + reg = <0x8c0000 0x6000>;
> > + clock-names = "m-ahb", "s-ahb";
> > + clocks = <&clock_gcc GCC_QUPV3_WRAP_0_M_AHB_CLK>,
> > + <&clock_gcc GCC_QUPV3_WRAP_0_S_AHB_CLK>;
> > + }
>
> Regards,
> Bjorn
^ permalink raw reply
* Re: [PATCH v2 0/7] Introduce GENI SE Controller Driver
From: Randy Dunlap @ 2018-01-19 18:32 UTC (permalink / raw)
To: Karthikeyan Ramasubramanian, corbet-T1hC0tSOHrs,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
wsa-z923LK4zBo2bacvFa/9K2g,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, jslaby-IBi9RG/b67k
In-Reply-To: <1515805547-22816-1-git-send-email-kramasub-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 01/12/2018 05:05 PM, Karthikeyan Ramasubramanian wrote:
> Generic Interface (GENI) firmware based Qualcomm Universal Peripheral (QUP)
> Wrapper is a next generation programmable module for supporting a wide
> range of serial interfaces like UART, SPI, I2C, I3C, etc. A single QUP
> module can provide upto 8 Serial Interfaces using its internal Serial
> Engines (SE). The protocol supported by each interface is determined by
> the firmware loaded to the Serial Engine.
>
> This patch series introduces GENI SE Driver to manage the GENI based QUP
> Wrapper and the common aspects of all SEs inside the QUP Wrapper. This
> patch series also introduces the UART and I2C Controller drivers to
> drive the SEs that are programmed with the respective protocols.
Hi,
Will there be follow-up drivers for SPI, I3C, etc.?
Thanks.
> [v2]
> * Updated device tree bindings to describe the hardware
> * Updated SE DT node as child node of QUP Wrapper DT node
> * Moved common AHB clocks to QUP Wrapper DT node
> * Use the standard "clock-frequency" I2C property
> * Update compatible field in UART Controller to reflect hardware manual
> * Addressed other device tree binding specific comments from Rob Herring
>
> Karthikeyan Ramasubramanian (7):
> qcom-geni-se: Add QCOM GENI SE Driver summary
> dt-bindings: soc: qcom: Add device tree binding for GENI SE
> soc: qcom: Add GENI based QUP Wrapper driver
> dt-bindings: i2c: Add device tree bindings for GENI I2C Controller
> i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
> controller
> dt-bindings: serial: Add bindings for GENI based UART Controller
> tty: serial: msm_geni_serial: Add serial driver support for GENI based
> QUP
>
> .../devicetree/bindings/i2c/i2c-qcom-geni.txt | 35 +
> .../devicetree/bindings/serial/qcom,geni-uart.txt | 29 +
> .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 66 +
> Documentation/qcom-geni-se.txt | 56 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-qcom-geni.c | 656 +++++++++
> drivers/soc/qcom/Kconfig | 8 +
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/qcom-geni-se.c | 1016 ++++++++++++++
> drivers/tty/serial/Kconfig | 10 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/qcom_geni_serial.c | 1414 ++++++++++++++++++++
> include/linux/qcom-geni-se.h | 807 +++++++++++
> 14 files changed, 4110 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-qcom-geni.txt
> create mode 100644 Documentation/devicetree/bindings/serial/qcom,geni-uart.txt
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> create mode 100644 Documentation/qcom-geni-se.txt
> create mode 100644 drivers/i2c/busses/i2c-qcom-geni.c
> create mode 100644 drivers/soc/qcom/qcom-geni-se.c
> create mode 100644 drivers/tty/serial/qcom_geni_serial.c
> create mode 100644 include/linux/qcom-geni-se.h
>
--
~Randy
--
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 v6 06/36] nds32: Kernel booting and initialization
From: Arnd Bergmann @ 2018-01-19 16:41 UTC (permalink / raw)
To: Greentime Hu
Cc: 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, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAEbi=3dJm=M-1UjUftb5qb0YOOsq92zzF86xeaVOhzULjvT3eA@mail.gmail.com>
On Fri, Jan 19, 2018 at 5:34 PM, Greentime Hu <green.hu@gmail.com> wrote:
> Hi, Arnd:
>
> 2018-01-18 18:11 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>>
>> I had not looked at this patch in enough detail earlier, sorry about
>> that. It should be
>> easy enough to fix though.
>>
>>> +#ifdef CONFIG_VGA_CONSOLE
>>> +struct screen_info screen_info;
>>> +#endif
>>
>> I would assume that you can't ever have a VGA console. Just drop all
>> the references
>> here and instead send a patch to the fbdev maintainer to add the dependency
>> at CONFIG_VGA_CONSOLE to prevent selecting it with nds32.
>
> I found it can be built pass for now because we disable it in defconfig.
> Should I send the patch in v7 series?
yes, I think that would be best.
>>> +
>>> +extern void __init early_init_devtree(void *params);
>>> +extern void __init early_trap_init(void);
>>
>> similarly, these are declared in include/linux/of_fdt.h
>>
>
> early_trap_init is a nds32 function. I will move it to nds32.h
Right, makes sense.
>>> +void calibrate_delay(void)
>>> +{
>>> + const int *val;
>>> + struct device_node *cpu = NULL;
>>> + cpu = of_find_compatible_node(NULL, NULL, "andestech,nds32v3");
>>> + val = of_get_property(cpu, "clock-frequency", NULL);
>>> + if (!val || !*val)
>>> + panic("no cpu 'clock-frequency' parameter in device tree");
>>> + loops_per_jiffy = be32_to_cpup(val) / HZ;
>>> + pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
>>> + loops_per_jiffy / (500000 / HZ),
>>> + (loops_per_jiffy / (5000 / HZ)) % 100, loops_per_jiffy);
>>> +}
>>
>> This seems very odd to me: The 'clock-frequency' property in the
>> cpu node should refer to the actual frequency it is running at, but that
>> tends to be different from the bogomips as needed by the ndelay()
>> function. Can you explain what is going on here?
>>
>
> This implementation is referenced from openrisc.
> https://lkml.org/lkml/2017/11/17/228
It's correct on openrisc, because that has a reliable cycle counter,
and that gets used in its delay function:
void __delay(unsigned long cycles)
{
cycles_t start = get_cycles();
while ((get_cycles() - start) < cycles)
cpu_relax();
}
In my review comment that you cited, I assumed that nds32 had similar
hardware.
However, as you explained earlier, the nds32 architecture does not provide
a cycle counter and the clocksource resolution is not high enough to
be a good replacement, so you have to use the traditional delay
calibration.
Arnd
^ permalink raw reply
* Re: [PATCH v6 06/36] nds32: Kernel booting and initialization
From: Greentime Hu @ 2018-01-19 16:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: 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, Geert Uytterhoeven,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a143yQ72+QGZSxpiFc7p8Hb7PXuCybJBFoLRSBaZ2uw4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi, Arnd:
2018-01-18 18:11 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> I had not looked at this patch in enough detail earlier, sorry about
> that. It should be
> easy enough to fix though.
>
>> +#ifdef CONFIG_VGA_CONSOLE
>> +struct screen_info screen_info;
>> +#endif
>
> I would assume that you can't ever have a VGA console. Just drop all
> the references
> here and instead send a patch to the fbdev maintainer to add the dependency
> at CONFIG_VGA_CONSOLE to prevent selecting it with nds32.
I found it can be built pass for now because we disable it in defconfig.
Should I send the patch in v7 series?
>> +
>> +extern void __init early_init_devtree(void *params);
>> +extern void __init early_trap_init(void);
>
> similarly, these are declared in include/linux/of_fdt.h
>
early_trap_init is a nds32 function. I will move it to nds32.h
>> +void __init setup_arch(char **cmdline_p)
>> +{
>> + early_init_devtree(__atags_pointer ?
>> + phys_to_virt(__atags_pointer) : __dtb_start);
>
> The reference to '__atags_pointer' appears to be a leftover from pre-DT
> days. Can you just remove that?
Yes, I will remove it.
>> +void calibrate_delay(void)
>> +{
>> + const int *val;
>> + struct device_node *cpu = NULL;
>> + cpu = of_find_compatible_node(NULL, NULL, "andestech,nds32v3");
>> + val = of_get_property(cpu, "clock-frequency", NULL);
>> + if (!val || !*val)
>> + panic("no cpu 'clock-frequency' parameter in device tree");
>> + loops_per_jiffy = be32_to_cpup(val) / HZ;
>> + pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
>> + loops_per_jiffy / (500000 / HZ),
>> + (loops_per_jiffy / (5000 / HZ)) % 100, loops_per_jiffy);
>> +}
>
> This seems very odd to me: The 'clock-frequency' property in the
> cpu node should refer to the actual frequency it is running at, but that
> tends to be different from the bogomips as needed by the ndelay()
> function. Can you explain what is going on here?
>
This implementation is referenced from openrisc.
https://lkml.org/lkml/2017/11/17/228
Thank you.
--
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 v6 31/36] dt-bindings: nds32 CPU Bindings
From: Geert Uytterhoeven @ 2018-01-19 15:37 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-u79uwXL29TY76Z2rM5mHXA,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren,
Randy Dunlap <rd>
In-Reply-To: <CAEbi=3fKp5tj32hoH=ZTo6kqpCE+Zv1LopnpVujusSfMkeaJKg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Greentime,
On Fri, Jan 19, 2018 at 4:35 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>>> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>>>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>>>
>>>>>>> This patch adds nds32 CPU binding documents.
>>>>>>>
>>>>>>> Signed-off-by: Vincent Chen <vincentc-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>>> Signed-off-by: Rick Chen <rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>>> Signed-off-by: Zong Li <zong-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>>> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>>> ---
>>>>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>>>>> 1 file changed, 37 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..9a52937
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>> @@ -0,0 +1,37 @@
>>>>>>> +* Andestech Processor Binding
>>>>>>> +
>>>>>>> +This binding specifies what properties must be available in the device tree
>>>>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>>>>> +tree.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> + - compatible:
>>>>>>> + Usage: required
>>>>>>> + Value type: <string>
>>>>>>> + Definition: should be one of:
>>>>>>> + "andestech,n13"
>>>>>>> + "andestech,n15"
>>>>>>> + "andestech,d15"
>>>>>>> + "andestech,n10"
>>>>>>> + "andestech,d10"
>>>>>>> + "andestech,nds32v3"
>>>>>>
>>>>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>>>>> the device tree should always list 'andestech,nds32v3' as the most
>>>>>> generic 'compatible' value and list exactly one of the others in
>>>>>> addition.
>>>>
>>>>> I will remove the others and just left "andestech,nds32v3" in here.
>>>>
>>>> No, is not what we want here, the CPU node should list exactly which core
>>>> is used, what we need in the description is a clarification that
>>>> andestech,nds32v3 must be used in addition to the more specific
>>>> string.
>>>
>>> Hi, Arnd:
>>>
>>> Sorry I still don't get your point. Do you mean we should always use
>>> compatible = "andestech,n13", "andestech,nds32v3";
>>> instead of
>>> compatible = "andestech,n13";
>>
>> Exactly. The first value is a device-specific compatible value, the second is
>> a generic fallback.
>>
>>> And I need to add the description in this document.
>>
>> Indeed. See for example
>> Documentation/devicetree/bindings/power/renesas,apmu.txt
>>
>> Thanks!
>
> Hi, Geert:
>
> Thank you and your example.
> I get it. I will update this document like this.
> - compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
> as fallback.
And please keep a list of supported values of "andestech,<core_name>"
in the DT binding document, so checkpatch can validate compatible values.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
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 v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-19 15:35 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, Linus Walleij,
Mark Rutland, Greg KH, Guo Ren, Randy Dunlap <rd>
In-Reply-To: <CAMuHMdX4nzU06ovDOwzJw-O8w0gyer-fi6skDae=K1yr-EN1gA@mail.gmail.com>
2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Greentime,
>
> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>>> From: Greentime Hu <greentime@andestech.com>
>>>>>>
>>>>>> This patch adds nds32 CPU binding documents.
>>>>>>
>>>>>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>>>>>> Signed-off-by: Rick Chen <rick@andestech.com>
>>>>>> Signed-off-by: Zong Li <zong@andestech.com>
>>>>>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>>>> 1 file changed, 37 insertions(+)
>>>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..9a52937
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>> @@ -0,0 +1,37 @@
>>>>>> +* Andestech Processor Binding
>>>>>> +
>>>>>> +This binding specifies what properties must be available in the device tree
>>>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>>>> +tree.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +
>>>>>> + - compatible:
>>>>>> + Usage: required
>>>>>> + Value type: <string>
>>>>>> + Definition: should be one of:
>>>>>> + "andestech,n13"
>>>>>> + "andestech,n15"
>>>>>> + "andestech,d15"
>>>>>> + "andestech,n10"
>>>>>> + "andestech,d10"
>>>>>> + "andestech,nds32v3"
>>>>>
>>>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>>>> the device tree should always list 'andestech,nds32v3' as the most
>>>>> generic 'compatible' value and list exactly one of the others in
>>>>> addition.
>>>
>>>> I will remove the others and just left "andestech,nds32v3" in here.
>>>
>>> No, is not what we want here, the CPU node should list exactly which core
>>> is used, what we need in the description is a clarification that
>>> andestech,nds32v3 must be used in addition to the more specific
>>> string.
>>
>> Hi, Arnd:
>>
>> Sorry I still don't get your point. Do you mean we should always use
>> compatible = "andestech,n13", "andestech,nds32v3";
>> instead of
>> compatible = "andestech,n13";
>
> Exactly. The first value is a device-specific compatible value, the second is
> a generic fallback.
>
>> And I need to add the description in this document.
>
> Indeed. See for example
> Documentation/devicetree/bindings/power/renesas,apmu.txt
>
> Thanks!
Hi, Geert:
Thank you and your example.
I get it. I will update this document like this.
- compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
as fallback.
^ permalink raw reply
* Re: [PATCH v6 31/36] dt-bindings: nds32 CPU Bindings
From: Geert Uytterhoeven @ 2018-01-19 15:29 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-u79uwXL29TY76Z2rM5mHXA,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren,
Randy Dunlap <rd>
In-Reply-To: <CAEbi=3eexCyrVhFZB8wmK1fT=QONzc63y6=dGNgj_H+Rt4zo_g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Greentime,
On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>>
>>>>> This patch adds nds32 CPU binding documents.
>>>>>
>>>>> Signed-off-by: Vincent Chen <vincentc-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>> Signed-off-by: Rick Chen <rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>> Signed-off-by: Zong Li <zong-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>> ---
>>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>>> 1 file changed, 37 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>> new file mode 100644
>>>>> index 0000000..9a52937
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>> @@ -0,0 +1,37 @@
>>>>> +* Andestech Processor Binding
>>>>> +
>>>>> +This binding specifies what properties must be available in the device tree
>>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>>> +tree.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible:
>>>>> + Usage: required
>>>>> + Value type: <string>
>>>>> + Definition: should be one of:
>>>>> + "andestech,n13"
>>>>> + "andestech,n15"
>>>>> + "andestech,d15"
>>>>> + "andestech,n10"
>>>>> + "andestech,d10"
>>>>> + "andestech,nds32v3"
>>>>
>>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>>> the device tree should always list 'andestech,nds32v3' as the most
>>>> generic 'compatible' value and list exactly one of the others in
>>>> addition.
>>
>>> I will remove the others and just left "andestech,nds32v3" in here.
>>
>> No, is not what we want here, the CPU node should list exactly which core
>> is used, what we need in the description is a clarification that
>> andestech,nds32v3 must be used in addition to the more specific
>> string.
>
> Hi, Arnd:
>
> Sorry I still don't get your point. Do you mean we should always use
> compatible = "andestech,n13", "andestech,nds32v3";
> instead of
> compatible = "andestech,n13";
Exactly. The first value is a device-specific compatible value, the second is
a generic fallback.
> And I need to add the description in this document.
Indeed. See for example
Documentation/devicetree/bindings/power/renesas,apmu.txt
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
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 v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-19 15:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: 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, Geert Uytterhoeven,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a38hCHULZt=fGXJebcoiEGCwA0fre_kixA7sUpPP2xf5g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>>
>>>> This patch adds nds32 CPU binding documents.
>>>>
>>>> Signed-off-by: Vincent Chen <vincentc-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>> Signed-off-by: Rick Chen <rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>> Signed-off-by: Zong Li <zong-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>> ---
>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>> 1 file changed, 37 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>> new file mode 100644
>>>> index 0000000..9a52937
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>> @@ -0,0 +1,37 @@
>>>> +* Andestech Processor Binding
>>>> +
>>>> +This binding specifies what properties must be available in the device tree
>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>> +tree.
>>>> +
>>>> +Required properties:
>>>> +
>>>> + - compatible:
>>>> + Usage: required
>>>> + Value type: <string>
>>>> + Definition: should be one of:
>>>> + "andestech,n13"
>>>> + "andestech,n15"
>>>> + "andestech,d15"
>>>> + "andestech,n10"
>>>> + "andestech,d10"
>>>> + "andestech,nds32v3"
>>>
>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>> the device tree should always list 'andestech,nds32v3' as the most
>>> generic 'compatible' value and list exactly one of the others in
>>> addition.
>
>> I will remove the others and just left "andestech,nds32v3" in here.
>
> No, is not what we want here, the CPU node should list exactly which core
> is used, what we need in the description is a clarification that
> andestech,nds32v3 must be used in addition to the more specific
> string.
Hi, Arnd:
Sorry I still don't get your point. Do you mean we should always use
compatible = "andestech,n13", "andestech,nds32v3";
instead of
compatible = "andestech,n13";
And I need to add the description in this document.
--
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 v6 31/36] dt-bindings: nds32 CPU Bindings
From: Arnd Bergmann @ 2018-01-19 14:52 UTC (permalink / raw)
To: Greentime Hu
Cc: 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, Geert Uytterhoeven,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAEbi=3ef8JHf_Jpru1B2L3+Xz-Oz4p_t84Lh=5kuTFo0YEz=5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>>
>>> This patch adds nds32 CPU binding documents.
>>>
>>> Signed-off-by: Vincent Chen <vincentc-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>> Signed-off-by: Rick Chen <rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>> Signed-off-by: Zong Li <zong-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> ---
>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>> 1 file changed, 37 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>> new file mode 100644
>>> index 0000000..9a52937
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>> @@ -0,0 +1,37 @@
>>> +* Andestech Processor Binding
>>> +
>>> +This binding specifies what properties must be available in the device tree
>>> +representation of a Andestech Processor Core, which is the root node in the
>>> +tree.
>>> +
>>> +Required properties:
>>> +
>>> + - compatible:
>>> + Usage: required
>>> + Value type: <string>
>>> + Definition: should be one of:
>>> + "andestech,n13"
>>> + "andestech,n15"
>>> + "andestech,d15"
>>> + "andestech,n10"
>>> + "andestech,d10"
>>> + "andestech,nds32v3"
>>
>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>> the device tree should always list 'andestech,nds32v3' as the most
>> generic 'compatible' value and list exactly one of the others in
>> addition.
> I will remove the others and just left "andestech,nds32v3" in here.
No, is not what we want here, the CPU node should list exactly which core
is used, what we need in the description is a clarification that
andestech,nds32v3 must be used in addition to the more specific
string.
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 v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-19 14:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: 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, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAK8P3a0HGhx3WpF-AwC+7m8+FoaiQBTYoCLqhECrATBRLX7x5g@mail.gmail.com>
2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> From: Greentime Hu <greentime@andestech.com>
>>
>> This patch adds nds32 CPU binding documents.
>>
>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>> Signed-off-by: Rick Chen <rick@andestech.com>
>> Signed-off-by: Zong Li <zong@andestech.com>
>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>> 1 file changed, 37 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>> new file mode 100644
>> index 0000000..9a52937
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>> @@ -0,0 +1,37 @@
>> +* Andestech Processor Binding
>> +
>> +This binding specifies what properties must be available in the device tree
>> +representation of a Andestech Processor Core, which is the root node in the
>> +tree.
>> +
>> +Required properties:
>> +
>> + - compatible:
>> + Usage: required
>> + Value type: <string>
>> + Definition: should be one of:
>> + "andestech,n13"
>> + "andestech,n15"
>> + "andestech,d15"
>> + "andestech,n10"
>> + "andestech,d10"
>> + "andestech,nds32v3"
>
> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
> the device tree should always list 'andestech,nds32v3' as the most
> generic 'compatible' value and list exactly one of the others in
> addition.
>
> Arnd
Hi, Arnd:
I will remove the others and just left "andestech,nds32v3" in here.
^ permalink raw reply
* Re: [PATCH v6 24/36] nds32: Loadable modules
From: Greentime Hu @ 2018-01-19 14:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: 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, Geert Uytterhoeven,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a3pQSxM+gJ+dJsyeo5YnuOOyFYVoJdCVmnoHR0Numya5g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-18 18:41 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>>
>> This patch adds support for loadable modules.
>
> One detail:
>
> You still seem to have both the ELF_REL and ELF_RELA based functions
> implemented here, you should drop the unused ELF_REL version:
>
>> diff --git a/arch/nds32/kernel/module.c b/arch/nds32/kernel/module.c
>> new file mode 100644
>> index 0000000..714a6d6
>> --- /dev/null
>> +++ b/arch/nds32/kernel/module.c
>> @@ -0,0 +1,286 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (C) 2005-2017 Andes Technology Corporation
>> +
>> +#include <linux/module.h>
>> +#include <linux/elf.h>
>> +#include <linux/vmalloc.h>
>> +
>> +#include <asm/pgtable.h>
>
> include <linux/moduleloader.h> to catch this.
>
>> +int
>> +apply_relocate(Elf32_Shdr * sechdrs, const char *strtab,
>> + unsigned int symindex, unsigned int relsec,
>> + struct module *module)
>> +{
>> + return 0;
>> +}
>
> and drop this.
>
> With that change,
>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Hi, Arnd:
Thank you. I will include moduleloader.h and drop apply_relocate().
--
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