Re: [PATCH v4 2/8] dt-bindings: ingenic: Add DT bindings for TCU clocks
Hi Paul,
Two things:
On Sun, Mar 18, 2018 at 12:28 AM, Paul Cercueil wrote:
> This header provides clock numbers for the ingenic,tcu
> DT binding.
I have tested the whole series on my Creator CI20 with success, using:
+ tcu@10002000 {
+ compatible = "ingenic,jz4780-tcu";
+ reg = <0x10002000 0x140>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <27 26 25>;
+ };
So:
Tested-by: Mathieu Malaterre
> Signed-off-by: Paul Cercueil
> Reviewed-by: Rob Herring
> ---
> include/dt-bindings/clock/ingenic,tcu.h | 23 +++
> 1 file changed, 23 insertions(+)
> create mode 100644 include/dt-bindings/clock/ingenic,tcu.h
>
> v2: Use SPDX identifier for the license
> v3: No change
> v4: No change
>
> diff --git a/include/dt-bindings/clock/ingenic,tcu.h
> b/include/dt-bindings/clock/ingenic,tcu.h
> new file mode 100644
> index ..179815d7b3bb
> --- /dev/null
> +++ b/include/dt-bindings/clock/ingenic,tcu.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides clock numbers for the ingenic,tcu DT binding.
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
> +#define __DT_BINDINGS_CLOCK_INGENIC_TCU_H__
> +
> +#define JZ4740_CLK_TIMER0 0
> +#define JZ4740_CLK_TIMER1 1
> +#define JZ4740_CLK_TIMER2 2
> +#define JZ4740_CLK_TIMER3 3
> +#define JZ4740_CLK_TIMER4 4
> +#define JZ4740_CLK_TIMER5 5
> +#define JZ4740_CLK_TIMER6 6
> +#define JZ4740_CLK_TIMER7 7
> +#define JZ4740_CLK_WDT 8
> +#define JZ4740_CLK_LASTJZ4740_CLK_WDT
> +
> +#define JZ4770_CLK_OST 9
> +#define JZ4770_CLK_LASTJZ4770_CLK_OST
> +
When working on JZ4780 support, I always struggle to read those kind
of #define. Since this is a new patch would you mind changing
s/JZ4740/JZ47XX/ in your header ?
Thanks for your work on JZ !
> +#endif /* __DT_BINDINGS_CLOCK_INGENIC_TCU_H__ */
> --
> 2.11.0
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 6/6] arm64: dts: sdm845: Add I2C controller support
Quoting Doug Anderson (2018-03-19 16:56:27) > On Mon, Mar 19, 2018 at 3:15 PM, Sagar Dharia wrote: > > > > Yes, we typically have a "platform(sdm845 here)-qupv3.dtsi" defining > > most of the serial-bus instances (i2c, spi, and uart with > > status=disabled) that we include from the common header. The boards > > enable instances they need. > > Will that be okay? > > Unless you really feel the need to put these in a separate file I'd > just put them straight in sdm845.dtsi. Yeah, it'll get big, but > that's OK by me. I _think_ this matches what Bjorn was suggesting on > previous device tree patches, but CCing him just in case. I'm > personally OK with whatever Bjorn and other folks with more Qualcomm > history would like. Upstream puts all SoC nodes in the SoC file. The split file by functional area method is to avoid merge conflicts in the SoC file and that doesn't make sense outside of the internal workflow. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] crypto: doc - clarify hash callbacks state machine
Add a note that it is perfectly legal to "abandon" a request object: - call .init() and then (as many times) .update() - _not_ call any of .final(), .finup() or .export() at any point in future Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au Signed-off-by: Horia Geantă --- Documentation/crypto/devel-algos.rst | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/crypto/devel-algos.rst b/Documentation/crypto/devel-algos.rst index 66f50d32dcec..c45c6f400dbd 100644 --- a/Documentation/crypto/devel-algos.rst +++ b/Documentation/crypto/devel-algos.rst @@ -236,6 +236,14 @@ when used from another part of the kernel. | '---> HASH2 +Note that it is perfectly legal to "abandon" a request object: +- call .init() and then (as many times) .update() +- _not_ call any of .final(), .finup() or .export() at any point in future + +In other words implementations should mind the resource allocation and clean-up. +No resources related to request objects should remain allocated after a call +to .init() or .update(), since there might be no chance to free them. + Specifics Of Asynchronous HASH Transformation ~ -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine
On 20.03.2018 08:56, Horia Geantă wrote: > Add a note that it is perfectly legal to "abandon" a request object: > - call .init() and then (as many times) .update() > - _not_ call any of .final(), .finup() or .export() at any point in > future > > Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au > Signed-off-by: Horia Geantă > --- > Documentation/crypto/devel-algos.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/crypto/devel-algos.rst > b/Documentation/crypto/devel-algos.rst > index 66f50d32dcec..c45c6f400dbd 100644 > --- a/Documentation/crypto/devel-algos.rst > +++ b/Documentation/crypto/devel-algos.rst > @@ -236,6 +236,14 @@ when used from another part of the kernel. > | > '---> HASH2 > > +Note that it is perfectly legal to "abandon" a request object: > +- call .init() and then (as many times) .update() > +- _not_ call any of .final(), .finup() or .export() at any point in future > + > +In other words implementations should mind the resource allocation and > clean-up. > +No resources related to request objects should remain allocated after a call -- > +to .init() or .update(), since there might be no chance to free them. is it for crypto api users or for drivers ? the creator of request context is responsible for alloc and destroy, so why there are no chance of free ? -- Best regards, Kamil Konieczny Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/8] dt-bindings: Add doc for the Ingenic TCU drivers
On 17/03/18 23:28, Paul Cercueil wrote:
> Add documentation about how to properly use the Ingenic TCU
> (Timer/Counter Unit) drivers from devicetree.
>
> Signed-off-by: Paul Cercueil
> ---
> .../bindings/clock/ingenic,tcu-clocks.txt | 42
> .../bindings/interrupt-controller/ingenic,tcu.txt | 39 +++
> .../devicetree/bindings/mfd/ingenic,tcu.txt| 56
> ++
> .../devicetree/bindings/timer/ingenic,tcu.txt | 41
> 4 files changed, 178 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
> create mode 100644
> Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
> create mode 100644 Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>
> v4: New patch in this series. Corresponds to V2 patches 3-4-5 with
> added content.
>
> diff --git a/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
> b/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
> new file mode 100644
> index ..471d27078599
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ingenic,tcu-clocks.txt
> @@ -0,0 +1,42 @@
> +Ingenic SoC TCU binding
> +
> +The TCU is the Timer/Counter Unit present in all Ingenic SoCs. It features 8
> +channels, each one having its own clock, that can be started and stopped,
> +reparented, and reclocked.
> +
> +Required properties:
> +- compatible : One of:
> + * ingenic,jz4740-tcu-clocks,
> + * ingenic,jz4770-tcu-clocks,
> + * ingenic,jz4780-tcu-clocks.
> +- clocks : List of phandle & clock specifiers for clocks external to the TCU.
> + The "pclk", "rtc" and "ext" clocks should be provided.
> +- clock-names : List of name strings for the external clocks.
> +- #clock-cells: Should be 1.
> + Clock consumers specify this argument to identify a clock. The valid values
> + may be found in .
> +
> +Example:
> +
> +/ {
> + tcu: mfd@10002000 {
> + compatible = "ingenic,tcu", "simple-mfd", "syscon";
> + reg = <0x10002000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x10002000 0x1000>;
> +
> + tcu_clk: clocks@10 {
> + compatible = "ingenic,jz4740-tcu-clocks";
> + reg = <0x10 0xff0>;
> +
> + clocks = <&ext>, <&rtc>, <&pclk>;
> + clock-names = "ext", "rtc", "pclk";
> +
> + #clock-cells = <1>;
> + };
> + };
> +};
> +
> +For information about the top-level "ingenic,tcu" compatible node and other
> +children nodes, see Documentation/devicetree/bindings/mfd/ingenic,tcu.txt.
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
> b/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
> new file mode 100644
> index ..7f3af2da77cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ingenic,tcu.txt
> @@ -0,0 +1,39 @@
> +Ingenic SoCs Timer/Counter Unit Interrupt Controller
> +
> +Required properties:
> +
> +- compatible : should be "ingenic,-tcu-intc". Valid strings are:
> + * ingenic,jz4740-tcu-intc
> + * ingenic,jz4770-tcu-intc
> + * ingenic,jz4780-tcu-intc
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The value shall be 1.
> +- interrupt-parent : phandle of the interrupt controller.
> +- interrupts : Specifies the interrupt the controller is connected to.
> +
> +Example:
> +
> +/ {
> + tcu: mfd@10002000 {
> + compatible = "ingenic,tcu", "simple-mfd", "syscon";
> + reg = <0x10002000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x10002000 0x1000>;
> +
> + tcu_irq: interrupt-controller@20 {
> + compatible = "ingenic,jz4740-tcu-intc";
> + reg = <0x20 0x20>;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interrupt-parent = <&intc>;
> + interrupts = <15>;
> + };
> + };
> +};
> +
> +For information about the top-level "ingenic,tcu" compatible node and other
> +children nodes, see Documentation/devicetree/bindings/mfd/ingenic,tcu.txt.
> diff --git a/Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
> b/Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
> new file mode 100644
> index ..5742c3f21550
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ingenic,tcu.txt
> @@ -0,0 +1,56 @@
> +Ingenic JZ47xx SoCs Timer/Counter Unit devicetree bindings
> +--
> +
> +For a description o
Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine
On 3/20/2018 10:50 AM, Kamil Konieczny wrote: > On 20.03.2018 08:56, Horia Geantă wrote: >> Add a note that it is perfectly legal to "abandon" a request object: >> - call .init() and then (as many times) .update() >> - _not_ call any of .final(), .finup() or .export() at any point in >> future >> >> Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au >> Signed-off-by: Horia Geantă >> --- >> Documentation/crypto/devel-algos.rst | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/Documentation/crypto/devel-algos.rst >> b/Documentation/crypto/devel-algos.rst >> index 66f50d32dcec..c45c6f400dbd 100644 >> --- a/Documentation/crypto/devel-algos.rst >> +++ b/Documentation/crypto/devel-algos.rst >> @@ -236,6 +236,14 @@ when used from another part of the kernel. >> | >> '---> HASH2 >> >> +Note that it is perfectly legal to "abandon" a request object: >> +- call .init() and then (as many times) .update() >> +- _not_ call any of .final(), .finup() or .export() at any point in future >> + >> +In other words implementations should mind the resource allocation and >> clean-up. >> +No resources related to request objects should remain allocated after a call > -- >> +to .init() or .update(), since there might be no chance to free them. > > is it for crypto api users or for drivers ? > For drivers / providers (below crypto API). > the creator of request context is responsible for alloc and destroy, > so why there are no chance of free ? > Hash request object (including request context) is allocated by the user / client by means of ahash_request_alloc(), and later on freed using ahash_request_free(). I don't see a problem with this. However, besides the memory allocated for the request context, other resources (related to the request) might be needed by the driver. I provided an example of needing to DMA map a buffer (to load/store HW state from/to crypto engine), and I am not happy with either solutions: -DMA map & unmap after each .update() -Herbert's proposal to use a convoluted DMA mapping scheme Another example: dynamic memory allocation might be needed beyond what's available in request context, i.e. driver might not have apriori all the information needed to inform the tfm about required memory using crypto_ahash_set_reqsize(). This happens due to the semantics of the crypto API, which allows the user to initialize a request object and drop it without getting a result (final or partial hash). I don't see what below use case is good for, maybe just for benchmarking: req = ahash_request_alloc(); [...] crypto_ahash_init(req); crypto_ahash_update(req); ahash_request_free(req); Horia -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] x86/cpu_hotplug: one bug fix and four cleanup
Recently, we hoped to make the possible CPU count more accurate for Kernel. I stuck on the issue how do I run acpi_early_init() _before_ setup_percpu() is invoked. So send these insignificant patches first. This patchset does this things: - two document-related work(the 1th and 2th patch), - two cleapup work (the 3th and 5th patch) - a bug fix for CPU hotplug(4th patch) Dou Liyang (5): x86/smpboot: Add the missing description of possible_cpus x86/cpu_hotplug: Update the link of cpu_hotplug.rst x86/smpboot: Make the check code more clear in prefill_possible_map() acpi/processor: Fix the return value of acpi_processor_ids_walk() acpi/processor: Make the acpi_duplicate_processor_id() static Documentation/00-INDEX | 2 - Documentation/admin-guide/kernel-parameters.txt | 5 ++ Documentation/cputopology.txt | 10 ++-- Documentation/x86/x86_64/cpu-hotplug-spec | 2 +- arch/x86/kernel/smpboot.c | 31 +++- drivers/acpi/acpi_processor.c | 66 - include/linux/acpi.h| 3 -- 7 files changed, 62 insertions(+), 57 deletions(-) -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
In prefill_possible_map(), Kernel need to get the number of possible CPUs
to reset cpu_possible_map. The number is related to the options:
-nosmp, maxcpus, possible_cpus, nr_cpus
... which need to be checked.
Currentlly, the check code mixed these options together in confusion and
hard to follow.
Isolate them, and check them linearly.
No functionary change, Prepare for cutting the number of possible CPUs
Signed-off-by: Dou Liyang
---
Situations:
setup_possible_cpus == -1 | setup_max_cpus == 0 | CONFIG_HOTPLUG_CPU == y |
yes | yes | yes |
no| no | no |
Test cases:
Cases | the number of possible cpus
| (the same with or w/o this patch)
case 1: no | no | no | --> min (setup_possible_cpus, nr_cpu_ids,
setup_max_cpus)
case 2: no | no | yes| --> min (setup_possible_cpus, nr_cpu_ids)
case 3: no | yes | no | --> 1
case 4: no | yes | yes| --> 1
case 5: yes | no | no | --> min (num_processors, nr_cpu_ids, setup_max_cpus)
case 6: yes | no | yes| --> min (num_processors + disabled_cpus, nr_cpu_ids)
case 7: yes | yes | no | --> 1
case 8: yes | yes | yes| --> 1
arch/x86/kernel/smpboot.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index ff99e2b6fc54..2fdda8567bf9 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1334,7 +1334,7 @@ early_param("possible_cpus", _setup_possible_cpus);
* We do this because additional CPUs waste a lot of memory.
* -AK
*/
-__init void prefill_possible_map(void)
+void __init prefill_possible_map(void)
{
int i, possible;
@@ -1356,18 +1356,22 @@ __init void prefill_possible_map(void)
num_processors = 1;
}
- i = setup_max_cpus ?: 1;
+ /* The SMP kernel could be acted as an UP kernel via nosmp/maxcpus=0 */
+ if (!setup_max_cpus) {
+ possible = 1;
+ total_cpus = num_processors + disabled_cpus;
+ goto set_cpu_possible_mask;
+ }
+
+ /* Possible CPUs could be overwrited via possible_cpus= */
if (setup_possible_cpus == -1) {
possible = num_processors;
#ifdef CONFIG_HOTPLUG_CPU
- if (setup_max_cpus)
- possible += disabled_cpus;
-#else
- if (possible > i)
- possible = i;
+ possible += disabled_cpus;
#endif
- } else
+ } else {
possible = setup_possible_cpus;
+ }
total_cpus = max_t(int, possible, num_processors + disabled_cpus);
@@ -1378,15 +1382,16 @@ __init void prefill_possible_map(void)
possible = nr_cpu_ids;
}
-#ifdef CONFIG_HOTPLUG_CPU
- if (!setup_max_cpus)
-#endif
- if (possible > i) {
+#ifndef CONFIG_HOTPLUG_CPU
+ /* Possible CPUs could be reduced via max_cpus= */
+ if (possible > setup_max_cpus) {
pr_warn("%d Processors exceeds max_cpus limit of %u\n",
possible, setup_max_cpus);
- possible = i;
+ possible = setup_max_cpus;
}
+#endif
+set_cpu_possible_mask:
nr_cpu_ids = possible;
pr_info("Allowing %d CPUs, %d hotplug CPUs\n",
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] acpi/processor: Make the acpi_duplicate_processor_id() static
The acpi_duplicate_processor_id() is only called in acpi_processor_get_info(),
So move it in front of acpi_processor_get_info() and make it static.
Signed-off-by: Dou Liyang
---
drivers/acpi/acpi_processor.c | 62 +--
include/linux/acpi.h | 3 ---
2 files changed, 31 insertions(+), 34 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index db5bdb59639c..03ec7690710c 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -229,6 +229,37 @@ static inline int acpi_processor_hotadd_init(struct
acpi_processor *pr)
}
#endif /* CONFIG_ACPI_HOTPLUG_CPU */
+/* The number of the unique processor IDs */
+static int nr_unique_ids __initdata;
+
+/* The number of the duplicate processor IDs */
+static int nr_duplicate_ids;
+
+/* Used to store the unique processor IDs */
+static int unique_processor_ids[] __initdata = {
+ [0 ... NR_CPUS - 1] = -1,
+};
+
+/* Used to store the duplicate processor IDs */
+static int duplicate_processor_ids[] = {
+ [0 ... NR_CPUS - 1] = -1,
+};
+
+static bool acpi_duplicate_processor_id(int proc_id)
+{
+ int i;
+
+ /*
+* Compare the proc_id with duplicate IDs, if the proc_id is already
+* in the duplicate IDs, return true, otherwise, return false.
+*/
+ for (i = 0; i < nr_duplicate_ids; i++) {
+ if (duplicate_processor_ids[i] == proc_id)
+ return true;
+ }
+ return false;
+}
+
static int acpi_processor_get_info(struct acpi_device *device)
{
union acpi_object object = { 0 };
@@ -579,22 +610,6 @@ static struct acpi_scan_handler
processor_container_handler = {
.attach = acpi_processor_container_attach,
};
-/* The number of the unique processor IDs */
-static int nr_unique_ids __initdata;
-
-/* The number of the duplicate processor IDs */
-static int nr_duplicate_ids;
-
-/* Used to store the unique processor IDs */
-static int unique_processor_ids[] __initdata = {
- [0 ... NR_CPUS - 1] = -1,
-};
-
-/* Used to store the duplicate processor IDs */
-static int duplicate_processor_ids[] = {
- [0 ... NR_CPUS - 1] = -1,
-};
-
static void __init processor_validated_ids_update(int proc_id)
{
int i;
@@ -682,21 +697,6 @@ static void __init acpi_processor_check_duplicates(void)
NULL, NULL);
}
-bool acpi_duplicate_processor_id(int proc_id)
-{
- int i;
-
- /*
-* compare the proc_id with duplicate IDs, if the proc_id is already
-* in the duplicate IDs, return true, otherwise, return false.
-*/
- for (i = 0; i < nr_duplicate_ids; i++) {
- if (duplicate_processor_ids[i] == proc_id)
- return true;
- }
- return false;
-}
-
void __init acpi_processor_init(void)
{
acpi_processor_check_duplicates();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 968173ec2726..dd4591dc1eb3 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -285,9 +285,6 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id)
return phys_id == PHYS_CPUID_INVALID;
}
-/* Validate the processor object's proc_id */
-bool acpi_duplicate_processor_id(int proc_id);
-
#ifdef CONFIG_ACPI_HOTPLUG_CPU
/* Arch dependent functions for cpu hotplug support */
int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
Kernel uses the possible_cpus in command line to reset the possible_cpus bits in cpu_possible_map. It doesn't be recorded in the kernel-parameters.txt Add its description in this document, also replace the wrong option additional_cpus with possible_cpus in cpu-gotplug-spec. Signed-off-by: Dou Liyang --- Documentation/admin-guide/kernel-parameters.txt | 5 + Documentation/x86/x86_64/cpu-hotplug-spec | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 1d1d53f85ddd..34f8a5f04e63 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2832,6 +2832,11 @@ variables need be pre-allocated for later physical cpu hot plugging. + possible_cpus= [s390,x86_64] Use this to set hotpluggable cpus. + This option sets possible_cpus bits in cpu_possible_map. + Thus keeping the numbers of bits set constant even if + the machine gets rebooted. + nr_uarts= [SERIAL] maximum number of UARTs to be registered. numa_balancing= [KNL,X86] Enable or disable automatic NUMA balancing. diff --git a/Documentation/x86/x86_64/cpu-hotplug-spec b/Documentation/x86/x86_64/cpu-hotplug-spec index 3c23e0587db3..d218bc0bdaaa 100644 --- a/Documentation/x86/x86_64/cpu-hotplug-spec +++ b/Documentation/x86/x86_64/cpu-hotplug-spec @@ -16,6 +16,6 @@ it should have its LAPIC Enabled bit set to 0. Linux will use the number of disabled LAPICs to compute the maximum number of future CPUs. In the worst case the user can overwrite this choice using a command line -option (additional_cpus=...), but it is recommended to supply the correct +option (possible_cpus=...), but it is recommended to supply the correct number (or a reasonable approximation of it, with erring towards more not less) in the MADT to avoid manual configuration. -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()
ACPI driver should make sure all the processor IDs in their ACPI Namespace
are unique for CPU hotplug. the driver performs a depth-first walk of the
namespace tree and calls the acpi_processor_ids_walk().
But, the acpi_processor_ids_walk() will return true if one processor is
checked, that cause the walk break after walking pass the first processor.
Repace the value with AE_OK which is the standard acpi_status value.
Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for processor
enumeration")
Signed-off-by: Dou Liyang
---
drivers/acpi/acpi_processor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 449d86d39965..db5bdb59639c 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -663,11 +663,11 @@ static acpi_status __init
acpi_processor_ids_walk(acpi_handle handle,
}
processor_validated_ids_update(uid);
- return true;
+ return AE_OK;
err:
acpi_handle_info(handle, "Invalid processor object\n");
- return false;
+ return AE_OK;
}
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] x86/cpu_hotplug: Update the link of cpu_hotplug.rst
The original cpu_hotplug.txt documents describing CPU hotplug support in
Linux kernel. it was moved in to core-api/ and renamed cpu_hotplug.rst.
Update it's link in other documents
Fixes: ff58fa7f556c ("Documentation: Update CPU hotplug and move it to
core-api")
Signed-off-by: Dou Liyang
---
Documentation/00-INDEX| 2 --
Documentation/cputopology.txt | 10 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 7f3a0728ccf2..3773c67ea9e5 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -104,8 +104,6 @@ core-api/
- documentation on kernel core components.
cpu-freq/
- info on CPU frequency and voltage scaling.
-cpu-hotplug.txt
- - document describing CPU hotplug support in the Linux kernel.
cpu-load.txt
- document describing how CPU load statistics are collected.
cpuidle/
diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
index c6e7e9196a8b..e05b0879fe91 100644
--- a/Documentation/cputopology.txt
+++ b/Documentation/cputopology.txt
@@ -117,9 +117,9 @@ source for the output is in brackets ("[]").
[NR_CPUS-1]
offline: CPUs that are not online because they have been
- HOTPLUGGED off (see cpu-hotplug.txt) or exceed the limit
- of CPUs allowed by the kernel configuration (kernel_max
- above). [~cpu_online_mask + cpus >= NR_CPUS]
+ HOTPLUGGED off (see core-api/cpu_hotplug.rst) or exceed
+ the limit of CPUs allowed by the kernel configuration
+ (kernel_max above). [~cpu_online_mask + cpus >= NR_CPUS]
online:CPUs that are online and being scheduled [cpu_online_mask]
@@ -155,5 +155,5 @@ online.)::
possible: 0-127
present: 0-3
-See cpu-hotplug.txt for the possible_cpus=NUM kernel start parameter
-as well as more information on the various cpumasks.
+See core-api/cpu_hotplug.rst for the possible_cpus=NUM kernel start
+parameter as well as more information on the various cpumasks.
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
On Tue, Mar 20, 2018 at 07:04:28PM +0800, Dou Liyang wrote: > + possible_cpus= [s390,x86_64] Use this to set hotpluggable cpus. > + This option sets possible_cpus bits in cpu_possible_map. > + Thus keeping the numbers of bits set constant even if > + the machine gets rebooted. That description, esp. the last sentence, doesn't make any kind of sense to me. What? -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
On Tue, Mar 20, 2018 at 07:04:30PM +0800, Dou Liyang wrote: > case 1: no | no | no | --> min (setup_possible_cpus, nr_cpu_ids, > setup_max_cpus) > case 2: no | no | yes| --> min (setup_possible_cpus, nr_cpu_ids) > case 3: no | yes | no | --> 1 > case 4: no | yes | yes| --> 1 > case 5: yes | no | no | --> min (num_processors, nr_cpu_ids, setup_max_cpus) > case 6: yes | no | yes| --> min (num_processors + disabled_cpus, nr_cpu_ids) > case 7: yes | yes | no | --> 1 > case 8: yes | yes | yes| --> 1 The case number is off by one ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy
On 03/19/2018 11:59 AM, Tejun Heo wrote:
> Hello, Waiman.
>
> This looks great. A couple nitpicks below.
>
>> + 5-3. Cpuset
>> + 5.3-1. Cpuset Interface Files
> Can we put cpuset below pid? It feels weird to break up cpu, memory
> and io as they represent the three major resources and are in a
> similar fashion.
Sure. I will move it down below pid.
>> + cpuset.effective_cpus
>> +A read-only multiple values file which exists on non-root
>> +cgroups.
>> +
>> +It lists the onlined CPUs that are actually allowed to be
>> +used by tasks within the current cgroup. It is a subset of
>> +"cpuset.cpus". Its value will be affected by CPU hotplug
>> +events.
> Can we do cpuset.cpus.availble which lists the cpus available to the
> cgroup instead of the eventual computed mask for the cgroup? That'd
> be more useful as it doesn't lose the information by and'ing what's
> available with the cgroup's mask and it's trivial to determine the
> effective from the two masks.
I don't get what you want here. cpus is the cpuset's cpus_allowed mask.
effective_cpus is the effective_cpus mask. When you say cpus available
to the cgroup, do you mean the cpu_online_mask or the list of cpus from
the parent? Or do you just want to change the name to cpus.available
instead of effective_cpus?
>> + cpuset.effective_mems
>> +A read-only multiple values file which exists on non-root
>> +cgroups.
>> +
>> +It lists the onlined memory nodes that are actually allowed
>> +to be used by tasks within the current cgroup. It is a subset
>> +of "cpuset.mems". Its value will be affected by memory nodes
>> +hotplug events.
> Ditto.
>
>> +static struct cftype dfl_files[] = {
>> +{
>> +.name = "cpus",
>> +.seq_show = cpuset_common_seq_show,
>> +.write = cpuset_write_resmask,
>> +.max_write_len = (100U + 6 * NR_CPUS),
>> +.private = FILE_CPULIST,
>> +},
> Is it missing CFTYPE_NOT_ON_ROOT? Other files too.
Right, I will set CFTYPE_NOT_ON_ROOT to "cpus" and "mems" as we are not
supposed to change them in the root. The effective_cpus and
effective_mems will be there in the root to show what are available.
Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] modify one dead link
Signed-off-by: Dongliang Mu --- Documentation/ia64/xen.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/ia64/xen.txt b/Documentation/ia64/xen.txt index a12c74ce2773..aea4bb94aa59 100644 --- a/Documentation/ia64/xen.txt +++ b/Documentation/ia64/xen.txt @@ -26,7 +26,7 @@ Getting and Building Xen and Dom0 DomainU OS : RHEL5 1. Download source -# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable.hg +# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable # cd xen-unstable.hg # hg clone http://xenbits.xensource.com/ext/ia64/linux-2.6.18-xen.hg -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:49)
> diff --git a/drivers/tty/serial/qcom_geni_serial.c
> b/drivers/tty/serial/qcom_geni_serial.c
> new file mode 100644
> index 000..1442777
> --- /dev/null
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -0,0 +1,1158 @@
> +
> +#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
> +static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch)
> +{
> + writel_relaxed(ch, uport->membase + SE_GENI_TX_FIFOn);
Does this expect the whole word to have data to write? Or does the FIFO
output a character followed by three NUL bytes each time it gets
written? The way that uart_console_write() works is to take each
character a byte at a time, put it into an int (so extend that byte with
zero) and then pass it to the putchar function. I would expect that at
this point the hardware sees the single character and then 3 NULs enter
the FIFO each time.
For previous MSM uarts I had to handle this oddity by packing the words
into the fifo four at a time. You may need to do the same here.
> +}
> +
> +static void
> +__qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
> +unsigned int count)
> +{
> + int i;
> + u32 bytes_to_send = count;
> +
> + for (i = 0; i < count; i++) {
> + if (s[i] == '\n')
Can you add a comment that uart_console_write() adds a carriage return
for each newline?
> + bytes_to_send++;
> + }
> +
> + writel_relaxed(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
> + qcom_geni_serial_setup_tx(uport, bytes_to_send);
> + for (i = 0; i < count; ) {
> + size_t chars_to_write = 0;
> + size_t avail = DEF_FIFO_DEPTH_WORDS - DEF_TX_WM;
> +
> + /*
> +* If the WM bit never set, then the Tx state machine is not
> +* in a valid state, so break, cancel/abort any existing
> +* command. Unfortunately the current data being written is
> +* lost.
> +*/
> + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_TX_FIFO_WATERMARK_EN, true))
> + break;
> + chars_to_write = min_t(size_t, (size_t)(count - i), avail /
> 2);
The _t part of min_t should do the casting already, so we can drop the
cast on count - i?
> + uart_console_write(uport, s + i, chars_to_write,
> + qcom_geni_serial_wr_char);
> + writel_relaxed(M_TX_FIFO_WATERMARK_EN, uport->membase +
> + SE_GENI_M_IRQ_CLEAR);
> + i += chars_to_write;
> + }
> + qcom_geni_serial_poll_tx_done(uport);
> +}
> +
> +static void qcom_geni_serial_console_write(struct console *co, const char *s,
> + unsigned int count)
> +{
> + struct uart_port *uport;
> + struct qcom_geni_serial_port *port;
> + bool locked = true;
> + unsigned long flags;
> +
> + WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
> +
> + port = get_port_from_line(co->index);
> + if (IS_ERR(port))
> + return;
> +
> + uport = &port->uport;
> + if (oops_in_progress)
> + locked = spin_trylock_irqsave(&uport->lock, flags);
> + else
> + spin_lock_irqsave(&uport->lock, flags);
> +
> + /* Cancel the current write to log the fault */
> + if (!locked) {
> + geni_se_cancel_m_cmd(&port->se);
> + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_CMD_CANCEL_EN, true)) {
> + geni_se_abort_m_cmd(&port->se);
> + qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_CMD_ABORT_EN, true);
> + writel_relaxed(M_CMD_ABORT_EN, uport->membase +
> + SE_GENI_M_IRQ_CLEAR);
> + }
> + writel_relaxed(M_CMD_CANCEL_EN, uport->membase +
> + SE_GENI_M_IRQ_CLEAR);
> + }
> +
> + __qcom_geni_serial_console_write(uport, s, count);
> + if (locked)
> + spin_unlock_irqrestore(&uport->lock, flags);
> +}
Can you also support the OF_EARLYCON_DECLARE method of console writing
so we can get an early printk style debug console?
> +
> +static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
> +{
> + u32 i;
> + unsigned char buf[sizeof(u32)];
> + struct tty_port *tport;
> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
> +
> + tport = &uport->state->port;
> + for (i = 0; i < bytes; ) {
> +
Re: [PATCH v4 1/6] dt-bindings: soc: qcom: Add device tree binding for GENI SE
Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:46) > Add device tree binding support for the QCOM GENI SE driver. > > Signed-off-by: Karthikeyan Ramasubramanian > Signed-off-by: Sagar Dharia > Signed-off-by: Girish Mahadevan Assuming Rob's comment is addressed: Reviewed-by: Stephen Boyd -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Non-DoD Source] Re: [PATCH v3 14/15] selinux: allow setxattr on rootfs so initramfs code can set them
On 03/10/2018 10:07 PM, Victor Kamensky wrote: > > > On Tue, 20 Feb 2018, Stephen Smalley wrote: > >> On Fri, 2018-02-16 at 20:33 +, Taras Kondratiuk wrote: >>> From: Victor Kamensky >>> >>> initramfs code supporting extended cpio format have ability to >>> fill extended attributes from cpio archive, but if SELinux enabled >>> and security server is not initialized yet, selinux callback would >>> refuse setxattr made by initramfs code. >>> >>> Solution enable SBLABEL_MNT on rootfs even if secrurity server is >>> not initialized yet. >> >> What if we were to instead skip the SBLABEL_MNT check in >> selinux_inode_setxattr() if !ss_initialized? Not dependent on >> filesystem type. > > Stephen, thank you for looking into this. Sorry, for dealyed reponse - > I needed to find time to require context about these changes. > > As you suggested I've tried this and it works: > >> From 6bf35bd055fdb12e94f3d5188eccfdbaa30dbcf4 Mon Sep 17 00:00:00 2001 > From: Victor Kamensky > Date: Fri, 9 Mar 2018 23:01:20 -0800 > Subject: [PATCH 1/2] selinux: allow setxattr on file systems if policy is not > loaded > > initramfs code supporting extended cpio format have ability to > fill extended attributes from cpio archive, but if SELinux enabled > and security server is not initialized yet, selinux callback would > refuse setxattr made by initramfs code because file system is not > yet marked as one that support labeling (SBLABEL_MNT flag). > > Solution do not refuse setxattr even if SBLABEL_MNT is not set > for file systems when policy is not loaded yet. > > Signed-off-by: Victor Kamensky > --- > security/selinux/hooks.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 819fd68..31303ed 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3120,7 +3120,7 @@ static int selinux_inode_setxattr(struct dentry > *dentry, const char *name, > return selinux_inode_setotherxattr(dentry, name); > > sbsec = inode->i_sb->s_security; > - if (!(sbsec->flags & SBLABEL_MNT)) > + if (!(sbsec->flags & SBLABEL_MNT) && ss_initialized) > return -EOPNOTSUPP; > > if (!inode_owner_or_capable(inode)) I favor the first option. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Non-DoD Source] Re: [PATCH v3 15/15] selinux: delay sid population for rootfs till init is complete
On 03/10/2018 10:08 PM, Victor Kamensky wrote:
>
>
> On Tue, 20 Feb 2018, Stephen Smalley wrote:
>
>> On Fri, 2018-02-16 at 20:33 +, Taras Kondratiuk wrote:
>>> From: Victor Kamensky
>>>
>>> With initramfs cpio format that supports extended attributes
>>> we need to skip sid population on sys_lsetxattr call from
>>> initramfs for rootfs if security server is not initialized yet.
>>>
>>> Otherwise callback in selinux_inode_post_setxattr will try to
>>> translate give security.selinux label into sid context and since
>>> security server is not available yet inode will receive default
>>> sid (typically kernel_t). Note that in the same time proper
>>> label will be stored in inode xattrs. Later, since inode sid
>>> would be already populated system will never look back at
>>> actual xattrs. But if we skip sid population for rootfs and
>>> we have policy that direct use of xattrs for rootfs, proper
>>> sid will be filled in from extended attributes one node is
>>> accessed and server is initialized.
>>>
>>> Note new DELAYAFTERINIT_MNT super block flag is introduced
>>> to only mark rootfs for such behavior. For other types of
>>> tmpfs original logic is still used.
>>
>> (cc selinux maintainers)
>>
>> Wondering if we shouldn't just do this always, for all filesystem
>> types.
>
> Ok, I think it makes sense. The one that do not support xattrs
> will not reach selinux_inode_post_setxattr anyway. And try
> to cache sid while !ss_initialized is not good idea for any
> filesystem types.
>
>> Also, I think this should likely also be done in
>> selinux_inode_setsecurity() for consistency.
>
> I am not sure that I follow selinux_inode_setsecurity suggestion.
> selinux_inode_setsecurity is about permission check. And
> selinux_inode_post_setxattr deals with processing and setting
> side effects if xattr was "security.selinux", it does not
> matter what happens in selinux_inode_setsecurity if it
> returns access_ok, LSM will still call selinux_inode_post_setxattr
> and we would need to check and not produce any sid caching
> side effects if !ss_initialized.
selinux_inode_setsecurity is the vfs fallback for setting security
attributes when the filesystem/inode does not support setxattr itself,
and is also used by kernfs.
So you need to update both selinux_inode_post_setxattr and
selinux_inode_setsecurity
in the same way.
>
> Sitll keeping logic in selinux_inode_post_setxattr, checked
> that the following with much simple code works too:
>
>> From bfc54e4805f3059671417ff2cda1266bc68e18f9 Mon Sep 17 00:00:00 2001
> From: Victor Kamensky
> Date: Fri, 9 Mar 2018 23:06:08 -0800
> Subject: [PATCH 2/2] selinux: delay sid population in setxattr till policy
> loaded
>
> With initramfs cpio format that supports extended attributes
> we need to skip sid population on sys_lsetxattr call from
> initramfs for rootfs if security server is not initialized yet.
>
> Otherwise callback in selinux_inode_post_setxattr will try to
> translate give security.selinux label into sid context and since
> security server is not available yet inode will receive default
> sid (typically kernel_t). Note that in the same time proper
> label will be stored in inode xattrs. Later, since inode sid
> would be already populated system will never look back at
> actual xattrs. But if we skip sid population for rootfs and
> we have policy that direct use of xattrs for rootfs, proper
> sid will be filled in from extended attributes one node is
> accessed and server is initialized.
>
> Signed-off-by: Victor Kamensky
> ---
> security/selinux/hooks.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 31303ed..4c13759 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3197,6 +3197,10 @@ static void selinux_inode_post_setxattr(struct dentry
> *dentry, const char *name,
> return;
> }
>
> + if (!ss_initialized) {
> + return;
> + }
> +
> rc = security_context_to_sid_force(value, size, &newsid);
> if (rc) {
> printk(KERN_ERR "SELinux: unable to map context to SID"
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] remove eight obsolete architectures
On Thu, 15 Mar 2018 03:42:25 PDT (-0700), Arnd Bergmann wrote: On Thu, Mar 15, 2018 at 10:59 AM, Hannes Reinecke wrote: On 03/15/2018 10:42 AM, David Howells wrote: Do we have anything left that still implements NOMMU? RISC-V ? (evil grin :-) Is anyone producing a chip that includes enough of the Privileged ISA spec to have things like system calls, but not the MMU parts? I thought at least initially the kernel only supports hardware that has a rather complete feature set. We currently do not have a NOMMU port. As far as I know, everyone who's currently producing RISC-V hardware with enough memory to run Linux has S mode with paging support. The ISA allows for S mode without paging but there's no hardware for that -- if you're going to put a DRAM controller on there then paging seems pretty cheap. You could run a NOMMU port on a system with S-mode and paging, but With all the superpage stuff I don't think you'll get an appreciable performance win for any workload running without an MMU so there's nothing to justify the work (and incompatibility) of a NOMMU port there. While I think you could implement a NOMMU port on a machine with only M and U modes (and therefor no address translation at all), I don't know of any MU-only machines that have enough memory to run Linux (ours have less than 32KiB). A SBI-free Linux would be a prerequisite for this, but there's some interest in that outside of a NOMMU port so it might materialize anyway. Of course, QEMU could probably be tricked into emulating one of these machines with little to no effort :)... That said, I doubt we'll see a NOMMU port materialize without some real hardware as it's a lot of work for a QEMU-only target. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
Hi Karthik,
On Wed, Mar 14, 2018 at 4:59 PM Karthikeyan Ramasubramanian <
krama...@codeaurora.org> wrote:
> +
> +static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
> + int offset, int field, bool set)
> +{
> + u32 reg;
> + struct qcom_geni_serial_port *port;
> + unsigned int baud;
> + unsigned int fifo_bits;
> + unsigned long timeout_us = 2;
> +
> + /* Ensure polling is not re-ordered before the prior writes/reads
*/
> + mb();
These barriers sprinkled around everywhere are new. Did
you find that you needed them for something? In this case, the
readl_poll_timeout_atomic should already have a read barrier (nor do you
probably care about read reordering, right?) Perhaps this should
only be a mmiowb rather than a full barrier, because you really just want
to say "make sure all my old writes got out to hardware before spinning"
> +
> + if (uport->private_data) {
> + port = to_dev_port(uport, uport);
> + baud = port->baud;
> + if (!baud)
> + baud = 115200;
> + fifo_bits = port->tx_fifo_depth * port->tx_fifo_width;
> + /*
> +* Total polling iterations based on FIFO worth of bytes
to be
> +* sent at current baud. Add a little fluff to the wait.
> +*/
> + timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
> + }
> +
> + return !readl_poll_timeout_atomic(uport->membase + offset, reg,
> +(bool)(reg & field) == set, 10, timeout_us);
> +}
[...]
> +
> +static void qcom_geni_serial_start_tx(struct uart_port *uport)
> +{
> + u32 irq_en;
> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
> + u32 status;
> +
> + if (port->xfer_mode == GENI_SE_FIFO) {
> + status = readl_relaxed(uport->membase + SE_GENI_STATUS);
> + if (status & M_GENI_CMD_ACTIVE)
> + return;
> +
> + if (!qcom_geni_serial_tx_empty(uport))
> + return;
> +
> + /*
> +* Ensure writing to IRQ_EN & watermark registers are not
> +* re-ordered before checking the status of the Serial
> +* Engine and TX FIFO
> +*/
> + mb();
Here's another one. You should just be able to use a regular readl when
reading SE_GENI_STATUS and remove this barrier, since readl has an rmb
which orders your read of M_IRQ_EN below. I don't think you need to worry
about the writes below going above the read above, since there's logic in
between that needs the result of the read. Maybe that also saves you in the
read case, too. Either way, this barrier seems heavy handed.
> +
> + irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
> + irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
> +
> + writel_relaxed(port->tx_wm, uport->membase +
> + SE_GENI_TX_WATERMARK_REG);
> + writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> + }
> +}
> +
> +static void qcom_geni_serial_stop_tx(struct uart_port *uport)
> +{
> + u32 irq_en;
> + u32 status;
> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
> +
> + irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
> + irq_en &= ~M_CMD_DONE_EN;
> + if (port->xfer_mode == GENI_SE_FIFO) {
> + irq_en &= ~M_TX_FIFO_WATERMARK_EN;
> + writel_relaxed(0, uport->membase +
> +SE_GENI_TX_WATERMARK_REG);
> + }
> + port->xmit_size = 0;
> + writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> + status = readl_relaxed(uport->membase + SE_GENI_STATUS);
> + /* Possible stop tx is called multiple times. */
> + if (!(status & M_GENI_CMD_ACTIVE))
> + return;
> +
> + /*
> +* Ensure cancel command write is not re-ordered before checking
> +* the status of the Primary Sequencer.
> +*/
> + mb();
I don't see how what's stated in your comment could happen, as that would
be a logic error. This barrier seems unneeded to me.
> +
> + geni_se_cancel_m_cmd(&port->se);
> + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_CMD_CANCEL_EN, true)) {
> + geni_se_abort_m_cmd(&port->se);
> + qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_CMD_ABORT_EN, true);
> + writel_relaxed(M_CMD_ABORT_EN, uport->membase +
> +
SE_GENI_M_IRQ_CLEAR);
> + }
> + writel_relaxed(M_CMD_CANCEL_EN, uport->membase +
SE_GENI_M_IRQ_CLEAR);
> +}
> +
> +static void qcom_geni_serial_start_rx(struct uart_port *up
Re: [PATCH v4 5/6] arm64: dts: sdm845: Add serial console support
Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:50)
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 979ab49..ea3efc5 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -12,4 +12,43 @@
> / {
> model = "Qualcomm Technologies, Inc. SDM845 MTP";
> compatible = "qcom,sdm845-mtp";
> +
> + aliases {
> + serial0 = &uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
Also add :115200n8 ?
> + };
> +};
> +
> +&soc {
I think the method is to put these inside soc node without using the
phandle reference. So indent everything once more.
> + geniqup@ac {
> + serial@a84000 {
> + status = "okay";
> + };
> + };
> +
> + pinctrl@340 {
> + qup-uart2-default {
> + pinconf_tx {
> + pins = "gpio4";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + pinconf_rx {
> + pins = "gpio5";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> + };
> +
> + qup-uart2-sleep {
> + pinconf {
> + pins = "gpio4", "gpio5";
> + bias-pull-down;
> + };
> + };
> + };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 32f8561..59334d9 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -6,6 +6,7 @@
> */
>
> #include
> +#include
>
> / {
> interrupt-parent = <&intc>;
> @@ -194,6 +195,20 @@
> #gpio-cells = <2>;
> interrupt-controller;
> #interrupt-cells = <2>;
> +
> + qup_uart2_default: qup-uart2-default {
> + pinmux {
> + function = "qup9";
> + pins = "gpio4", "gpio5";
> + };
> + };
> +
> + qup_uart2_sleep: qup-uart2-sleep {
> + pinmux {
> + function = "gpio";
> + pins = "gpio4", "gpio5";
> + };
> + };
Are these supposed to go to the board file?
> };
>
> timer@17c9 {
> @@ -272,5 +287,28 @@
> #interrupt-cells = <4>;
> cell-index = <0>;
> };
> +
> + geniqup@ac {
> + compatible = "qcom,geni-se-qup";
> + reg = <0xac 0x6000>;
> + clock-names = "m-ahb", "s-ahb";
> + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
> +<&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
Add a status = "disabled" here too.
> +
> + uart2: serial@a84000 {
> + compatible = "qcom,geni-debug-uart";
> + reg = <0xa84000 0x4000>;
> + clock-names = "se";
> + clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>;
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&qup_uart2_default>;
> + pinctrl-1 = <&qup_uart2_sleep>;
> + interrupts = IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> + };
> };
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] modify one dead link
On Tue, 20 Mar 2018 10:54:22 -0400 Dongliang Mu wrote: > -# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable.hg > +# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable > # cd xen-unstable.hg > # hg clone http://xenbits.xensource.com/ext/ia64/linux-2.6.18-xen.hg You will need to fix a 'cd' as well: cd xen-unstable Otherwise looks good. -- Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] modify one dead link
On 03/20/2018 03:42 PM, Sergei Trofimovich wrote: On Tue, 20 Mar 2018 10:54:22 -0400 Dongliang Mu wrote: -# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable.hg +# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable # cd xen-unstable.hg # hg clone http://xenbits.xensource.com/ext/ia64/linux-2.6.18-xen.hg You will need to fix a 'cd' as well: cd xen-unstable I will modify that problem. Then resubmit the patch. Thanks. Otherwise looks good. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fix one dead link in ia64/xen.txt
Signed-off-by: Dongliang Mu --- Documentation/ia64/xen.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/ia64/xen.txt b/Documentation/ia64/xen.txt index a12c74ce2773..464d4c29b8b5 100644 --- a/Documentation/ia64/xen.txt +++ b/Documentation/ia64/xen.txt @@ -26,8 +26,8 @@ Getting and Building Xen and Dom0 DomainU OS : RHEL5 1. Download source -# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable.hg -# cd xen-unstable.hg +# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable +# cd xen-unstable # hg clone http://xenbits.xensource.com/ext/ia64/linux-2.6.18-xen.hg 2. # make world -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy
Hello, Waiman. On Tue, Mar 20, 2018 at 09:51:20AM -0400, Waiman Long wrote: > >> + It lists the onlined CPUs that are actually allowed to be > >> + used by tasks within the current cgroup. It is a subset of > >> + "cpuset.cpus". Its value will be affected by CPU hotplug > >> + events. > > Can we do cpuset.cpus.availble which lists the cpus available to the > > cgroup instead of the eventual computed mask for the cgroup? That'd > > be more useful as it doesn't lose the information by and'ing what's > > available with the cgroup's mask and it's trivial to determine the > > effective from the two masks. > > I don't get what you want here. cpus is the cpuset's cpus_allowed mask. > effective_cpus is the effective_cpus mask. When you say cpus available > to the cgroup, do you mean the cpu_online_mask or the list of cpus from > the parent? Or do you just want to change the name to cpus.available > instead of effective_cpus? The available cpus from the parent, where the effective is AND between cpuset.available and cpuset.cpus of the cgroup, so that the user can see what's available for the cgroup unfiltered by cpuset.cpus. > Right, I will set CFTYPE_NOT_ON_ROOT to "cpus" and "mems" as we are not > supposed to change them in the root. The effective_cpus and > effective_mems will be there in the root to show what are available. So, we can do that in the future but let's not do that for now. It's the same problem we have for basically everything else and we've stayed away from replicating the information in the root cgroup. This might change in the future but if we do that let's do that consistently. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] cpuset: Add cpuset.flags control knob to v2
On 03/19/2018 12:33 PM, Waiman Long wrote: > On 03/19/2018 12:26 PM, Tejun Heo wrote: >> Hello, Waiman. >> >> On Thu, Mar 15, 2018 at 05:20:42PM -0400, Waiman Long wrote: >>> + The currently supported flag is: >>> + >>> + sched_load_balance >>> + When it is not set, there will be no load balancing >>> + among CPUs on this cpuset. Tasks will stay in the >>> + CPUs they are running on and will not be moved to >>> + other CPUs. >>> + >>> + When it is set, tasks within this cpuset will be >>> + load-balanced by the kernel scheduler. Tasks will be >>> + moved from CPUs with high load to other CPUs within >>> + the same cpuset with less load periodically. >> Hmm... looks like this is something which can be decided by the cgroup >> itself and should be made delegatable. Given that different flags >> might need different delegation settings and the precedence of >> memory.oom_group, I think it'd be better to make the flags separate >> bool files - ie. cpuset.sched_load_balance which contains 0/1 and >> marked delegatable. >> >> Thanks. >> > Sure. Will do that. After some thought, I am planning to impose the following additional constraints on how sched_load_balance works in v2. 1) sched_load_balance will be made hierarchical, the child will inherit the flag from its parent. 2) cpu_exclusive will be implicitly associated with sched_load_balance. IOW, sched_load_balance => !cpu_exclusive, and !sched_load_balance => cpu_exclusive. 3) sched_load_balance cannot be 1 on a child if it is 0 on the parent. With these changes, sched_load_balance will have to be set by the parent and so will not be delegatable. Please let me know your thought on that. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix one dead link in ia64/xen.txt
On 20/03/18 19:56, Dongliang Mu wrote: > Signed-off-by: Dongliang Mu > --- > Documentation/ia64/xen.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/ia64/xen.txt b/Documentation/ia64/xen.txt > index a12c74ce2773..464d4c29b8b5 100644 > --- a/Documentation/ia64/xen.txt > +++ b/Documentation/ia64/xen.txt > @@ -26,8 +26,8 @@ Getting and Building Xen and Dom0 > DomainU OS : RHEL5 > > 1. Download source > -# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable.hg > -# cd xen-unstable.hg > +# hg clone http://xenbits.xensource.com/ext/ia64/xen-unstable > +# cd xen-unstable > # hg clone http://xenbits.xensource.com/ext/ia64/linux-2.6.18-xen.hg > > 2. # make world The last commit in that repository is almost 9 years old, and IA64 support was dropped from Xen mainline6 years ago. http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=570c311ca2c7a1131570cdfc77e977bc7a9bf4c0 There are a number of other dead links in this doc, and those which aren't dead refer to Linux 2.6.x. I'd just remove the entire file, rather than pretend that any of this still might work. (If by some miracle it does still function, its 10 years behind on security fixes...) ~Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] cpuset: Add cpuset.flags control knob to v2
Hello, Waiman. On Tue, Mar 20, 2018 at 04:12:25PM -0400, Waiman Long wrote: > After some thought, I am planning to impose the following additional > constraints on how sched_load_balance works in v2. > > 1) sched_load_balance will be made hierarchical, the child will inherit > the flag from its parent. > 2) cpu_exclusive will be implicitly associated with sched_load_balance. > IOW, sched_load_balance => !cpu_exclusive, and !sched_load_balance => > cpu_exclusive. > 3) sched_load_balance cannot be 1 on a child if it is 0 on the parent. > > With these changes, sched_load_balance will have to be set by the parent > and so will not be delegatable. Please let me know your thought on that. So, for configurations, we usually don't let them interact across hierarchy because that can lead to configurations surprise-changing and delegated children locking the parent into the current config. This case could be different and as long as we always guarantee that an ancestor isn't limited by its descendants in what it can configure, it should be okay (e.g. an ancestor should always be able to turn on sched_load_balance regardless of how the descendants are configured). Hmmm... can you explain why sched_load_balance needs to behave this way? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] cpuset: Add cpuset.flags control knob to v2
On 03/20/2018 04:22 PM, Tejun Heo wrote: > Hello, Waiman. > > On Tue, Mar 20, 2018 at 04:12:25PM -0400, Waiman Long wrote: >> After some thought, I am planning to impose the following additional >> constraints on how sched_load_balance works in v2. >> >> 1) sched_load_balance will be made hierarchical, the child will inherit >> the flag from its parent. >> 2) cpu_exclusive will be implicitly associated with sched_load_balance. >> IOW, sched_load_balance => !cpu_exclusive, and !sched_load_balance => >> cpu_exclusive. >> 3) sched_load_balance cannot be 1 on a child if it is 0 on the parent. >> >> With these changes, sched_load_balance will have to be set by the parent >> and so will not be delegatable. Please let me know your thought on that. > So, for configurations, we usually don't let them interact across > hierarchy because that can lead to configurations surprise-changing > and delegated children locking the parent into the current config. > > This case could be different and as long as we always guarantee that > an ancestor isn't limited by its descendants in what it can configure, > it should be okay (e.g. an ancestor should always be able to turn on > sched_load_balance regardless of how the descendants are configured). Yes, I will do some testing to make sure that a descendant won't be able to affect how the ancestors can behave. > Hmmm... can you explain why sched_load_balance needs to behave this > way? It boils down to the fact that it doesn't make sense to have a CPU in an isolated cpuset to participate in load balancing in another cpuset as Mike has said before. It is especially true in a parent-child relationship where a delegatee can escape CPU isolation by re-enabling sched_load_balance in a child cpuset. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy
On 03/20/2018 04:10 PM, Tejun Heo wrote: > Hello, Waiman. > > On Tue, Mar 20, 2018 at 09:51:20AM -0400, Waiman Long wrote: + It lists the onlined CPUs that are actually allowed to be + used by tasks within the current cgroup. It is a subset of + "cpuset.cpus". Its value will be affected by CPU hotplug + events. >>> Can we do cpuset.cpus.availble which lists the cpus available to the >>> cgroup instead of the eventual computed mask for the cgroup? That'd >>> be more useful as it doesn't lose the information by and'ing what's >>> available with the cgroup's mask and it's trivial to determine the >>> effective from the two masks. >> I don't get what you want here. cpus is the cpuset's cpus_allowed mask. >> effective_cpus is the effective_cpus mask. When you say cpus available >> to the cgroup, do you mean the cpu_online_mask or the list of cpus from >> the parent? Or do you just want to change the name to cpus.available >> instead of effective_cpus? > The available cpus from the parent, where the effective is AND between > cpuset.available and cpuset.cpus of the cgroup, so that the user can > see what's available for the cgroup unfiltered by cpuset.cpus. ASAIK for v2, when cpuset.cpus is empty, cpuset.effective_cpus will show all the cpus available from the parent. It is a different behavior from v1. So do we still need a cpuset.cpus_available? >> Right, I will set CFTYPE_NOT_ON_ROOT to "cpus" and "mems" as we are not >> supposed to change them in the root. The effective_cpus and >> effective_mems will be there in the root to show what are available. > So, we can do that in the future but let's not do that for now. It's > the same problem we have for basically everything else and we've > stayed away from replicating the information in the root cgroup. This > might change in the future but if we do that let's do that > consistently. That is fine. I will make them all disappears in the root cgroup. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy
Hello, On Tue, Mar 20, 2018 at 04:53:37PM -0400, Waiman Long wrote: > ASAIK for v2, when cpuset.cpus is empty, cpuset.effective_cpus will show > all the cpus available from the parent. It is a different behavior from > v1. So do we still need a cpuset.cpus_available? Heh, you're right. Let's forget about available and do cpuset.cpus.effective. The primary reason for suggesting that was because of the similarity with cgroup.controllers and cgroup.subtree_control; however, they're that way because subtree_control is delegatable. For a normal resource knob like cpuset.cpus, the knob is owned by the parent and what's interesting to the parent is its effective set that it's distributing from. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy
On 03/20/2018 05:14 PM, Tejun Heo wrote: > Hello, > > On Tue, Mar 20, 2018 at 04:53:37PM -0400, Waiman Long wrote: >> ASAIK for v2, when cpuset.cpus is empty, cpuset.effective_cpus will show >> all the cpus available from the parent. It is a different behavior from >> v1. So do we still need a cpuset.cpus_available? > Heh, you're right. Let's forget about available and do > cpuset.cpus.effective. The primary reason for suggesting that was > because of the similarity with cgroup.controllers and > cgroup.subtree_control; however, they're that way because > subtree_control is delegatable. For a normal resource knob like > cpuset.cpus, the knob is owned by the parent and what's interesting to > the parent is its effective set that it's distributing from. OK, will change the names as suggested. -Longman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
On 3/19/2018 3:08 PM, Doug Anderson wrote:
> Hi,
>
> On Wed, Mar 14, 2018 at 4:58 PM, Karthikeyan Ramasubramanian
> wrote:
>> This bus driver supports the GENI based i2c hardware controller in the
>> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
>> module supporting a wide range of serial interfaces including I2C. The
>> driver supports FIFO mode and DMA mode of transfer and switches modes
>> dynamically depending on the size of the transfer.
>>
>> Signed-off-by: Karthikeyan Ramasubramanian
>> Signed-off-by: Sagar Dharia
>> Signed-off-by: Girish Mahadevan
>> ---
>> drivers/i2c/busses/Kconfig | 13 +
>> drivers/i2c/busses/Makefile| 1 +
>> drivers/i2c/busses/i2c-qcom-geni.c | 648
>> +
>> 3 files changed, 662 insertions(+)
>
> Since I reviewed v3 of the i2c patch, can you make sure that you CC me
> on future patches? Thanks! :)
>
> Overall this looks pretty nice to me now. A few minor comments...
Sorry for not keeping you in CC for this patch. I will keep you in the
CC going forward.
>
>
>> +static void geni_i2c_abort_xfer(struct geni_i2c_dev *gi2c)
>> +{
>> + u32 val;
>> + unsigned long time_left = ABORT_TIMEOUT;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gi2c->lock, flags);
>> + geni_i2c_err(gi2c, GENI_TIMEOUT);
>> + gi2c->cur = NULL;
>> + geni_se_abort_m_cmd(&gi2c->se);
>> + spin_unlock_irqrestore(&gi2c->lock, flags);
>> + do {
>> + time_left = wait_for_completion_timeout(&gi2c->done,
>> time_left);
>> + val = readl_relaxed(gi2c->se.base + SE_GENI_M_IRQ_STATUS);
>> + } while (!(val & M_CMD_ABORT_EN) && time_left);
>> +
>> + if (!(val & M_CMD_ABORT_EN) && !time_left)
>
> Why do you need to check !time_left? Just "if (!(val & M_CMD_ABORT_EN))".
I will remove here and in the other places you mentioned.
>
>
>> +
>> + gi2c->se.dev = &pdev->dev;
>> + gi2c->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + gi2c->se.base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(gi2c->se.base))
>> + return PTR_ERR(gi2c->se.base);
>> +
>> + gi2c->se.clk = devm_clk_get(&pdev->dev, "se");
>> + if (IS_ERR(gi2c->se.clk)) {
>> + ret = PTR_ERR(gi2c->se.clk);
>> + dev_err(&pdev->dev, "Err getting SE Core clk %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = device_property_read_u32(&pdev->dev, "clock-frequency",
>> + &gi2c->clk_freq_out);
>> + if (ret) {
>> + /* All GENI I2C slaves support 400kHz. So default to 400kHz.
>> */
>
> Can you explain this comment? Are you saying that GENI only supports
> talking to I2C devices (devices are also known as "slaves" and GENI
> should be the I2C master) that talk 400 kHz I2C or better? Why do you
> even have 100 kHz in your table above then? I don't think this is
> what you meant...
>
> Perhaps you meant to say "All GENI I2C masters support at least 400
> kHz, so default ot 400 kHz"
>
> ...however, even if you changed the comment as I suggested I'm still
> not a fan. As I said in my review of the prevous version:
>
>> I feel like it should default to 100KHz. i2c_parse_fw_timings()
>> defaults to this and to me the wording "New drivers almost always
>> should use the defaults" makes me feel this should be the defaults.
>
> I would also say that even if all GENI I2C masters support at least
> 400 kHz that doesn't mean that all I2C devices on the bus support 400
> kHz. You could certainly imagine someone sticking something on this
> bus that was super low cost and supported only 100 kHz I2C.
We felt that all the current slaves that are connected to our masters
support 400kHz. Hence we used that as a default frequency.
I agree with you regarding 100kHz as default frequency. I will update
that way in the next patch.
>
>
> -Doug
>
Regards,
Karthik.
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 6/6] arm64: dts: sdm845: Add I2C controller support
Hi,
On 3/19/2018 5:56 PM, Doug Anderson wrote:
> Hi,
>
> On Mon, Mar 19, 2018 at 3:15 PM, Sagar Dharia wrote:
+ pinconf {
+ pins = "gpio55", "gpio56";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
+ qup-i2c10-sleep {
+ pinconf {
+ pins = "gpio55", "gpio56";
+ bias-pull-up;
>>>
>>> Are you sure that you want pullups enabled for sleep here? There are
>>> external pulls on this line (as there are on many i2c busses) so doing
>>> this will double-enable pulls. It probably won't hurt, but I'm
>>> curious if there's some sort of reason here.
>>>
>> 1. We need the lines to remain high to avoid slaves sensing a false
>> start-condition (this can happen if the SDA goes down before SCL).
>> 2. Disclaimer: I'm not a HW expert, but we were told that
>> tri-state/bias-disabled lines can draw more current. I will find out
>> more about that.
>
> Agreed that they need to remain high, but you've got very strong
> pullups external to the SoC. Those will keep it high. You don't need
> the internal ones too.
>
> As extra evidence that the external pullups _must_ be present on your
> board: you specify bias-disable in the active state. That can only
> work if there are external pullups (or if there were some special
> extra secret internal pullups that were part of geni). i2c is an
> open-drain bus and thus there must be pullups on the bus in order to
> communicate.
>
You are right, I followed up about the pull-up recommendation and that
was for a GPIO where there was no external pull-up (GPIO was not used
for I2C). It's safe to assume I2C will always have external pullup. We
will change sleep-config of I2C GPIOs to no-pull.
>
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 59334d9..9ef056f 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -209,6 +209,21 @@
pins = "gpio4", "gpio5";
};
};
+
+ qup_i2c10_default: qup-i2c10-default {
+ pinmux {
+ function = "qup10";
+ pins = "gpio55", "gpio56";
+ };
+ };
+
+ qup_i2c10_sleep: qup-i2c10-sleep {
+ pinmux {
+ function = "gpio";
+ pins = "gpio55", "gpio56";
+ };
+ };
+
};
timer@17c9 {
@@ -309,6 +324,20 @@
interrupts = >>> IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};
+
+ i2c10: i2c@a88000 {
>>>
>>> Seems like it might be nice to add all the i2c busses into the main
>>> sdm845.dtsi file. Sure, most won't be enabled, but it seems like it
>>> would avoid churn later.
>>>
>>> ...if you're sure you want to add only one i2c controller, subject of
>>> this patch should indicate that.
>>>
>>
>> Yes, we typically have a "platform(sdm845 here)-qupv3.dtsi" defining
>> most of the serial-bus instances (i2c, spi, and uart with
>> status=disabled) that we include from the common header. The boards
>> enable instances they need.
>> Will that be okay?
>
> Unless you really feel the need to put these in a separate file I'd
> just put them straight in sdm845.dtsi. Yeah, it'll get big, but
> that's OK by me. I _think_ this matches what Bjorn was suggesting on
> previous device tree patches, but CCing him just in case. I'm
> personally OK with whatever Bjorn and other folks with more Qualcomm
> history would like.
>
> ...but yeah, I'm asking for them all to be listed with status="disabled".
>
Sure, we will change the subject of this patch to indicate that we are
adding 1 controller as of now. Later we will add all I2C controllers to
dtsi as another patch since that will need pinctrl settings for GPIOs
used by those instances and the wrappers devices needed by them.
Thanks
Sagar
>
> -Doug
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of the Code Auror
Re: [PATCH v4 3/6] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
Hi Doug,
On 3/19/2018 3:08 PM, Doug Anderson wrote:
> Hi,
>
> On Wed, Mar 14, 2018 at 4:58 PM, Karthikeyan Ramasubramanian
> wrote:
>> This bus driver supports the GENI based i2c hardware controller in the
>> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
>> module supporting a wide range of serial interfaces including I2C. The
>> driver supports FIFO mode and DMA mode of transfer and switches modes
>> dynamically depending on the size of the transfer.
>>
>> Signed-off-by: Karthikeyan Ramasubramanian
>> Signed-off-by: Sagar Dharia
>> Signed-off-by: Girish Mahadevan
>
>> +/*
>> + * Hardware uses the underlying formula to calculate time periods of
>> + * SCL clock cycle.
>> + *
>> + * time of high period of SCL: t_high = (t_high_cnt * clk_div) /
>> source_clock
>> + * time of low period of SCL: t_low = (t_low_cnt * clk_div) / source_clock
>> + * time of full period of SCL: t_cycle = (t_cycle_cnt * clk_div) /
>> source_clock
>> + * clk_freq_out = t / t_cycle
>> + * source_clock = 19.2 MHz
>> + */
>> +static const struct geni_i2c_clk_fld geni_i2c_clk_map[] = {
>> + {KHZ(100), 7, 10, 11, 26},
>> + {KHZ(400), 2, 5, 12, 24},
>> + {KHZ(1000), 1, 3, 9, 18},
>> +};
>
> Thanks for the docs. ...though if these docs are indeed correct and
> there's no extra magic fudge factor I'm still a bit baffled why the
> frequency isn't out of spec for 100 kHz and 1 MHz. I know you said
> hardware validated it, but are you really certain they validated 100
> kHz and 1 MHz?
>
19200. / (1 * 18)
> 1066.7
>
19200. / (2 * 24)
> 400.0
>
19200. / (7 * 26)
> 105.49450549450549
>
> Specifically: either the docs you wrote above are wrong (and there's a
> magic fudge factor that you didn't document) or your hardware guys are
> wrong. See the I2C spec at
> . Table 10.
> ("Characteristics of the SDA and SCL bus lines for Standard, Fast, and
> Fast-mode Plus I2C-bus devices") says fSCL Max is 100 kHz, 400 kHz, or
> 1000 kHz.
>
>
> I could believe that 99.9% of all devices that support 100 kHz also
> support 105.5 kHz and that 99.9% of all devices that support 1000 kHz
> also support 1066.7 kHz. However, it's still not in spec. If you
> want to enable a turbo boost mode that runs 5% faster (really?) then I
> suppose you could add that as an optional feature, but this shouldn't
> be the default.
>
Yes, we will document that there is a fudge-factor (cycles needed to run
the firmware per inputs from HW team). The actual frequencies seen for
100KHz and 1MHz configurations were close to 98KHz, and 960KHz
respectively. t-high, and t-low were within specs for all frequencies.
Thanks
Sagar
>
>> +static void geni_i2c_abort_xfer(struct geni_i2c_dev *gi2c)
>> +{
>> + u32 val;
>> + unsigned long time_left = ABORT_TIMEOUT;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&gi2c->lock, flags);
>> + geni_i2c_err(gi2c, GENI_TIMEOUT);
>> + gi2c->cur = NULL;
>> + geni_se_abort_m_cmd(&gi2c->se);
>> + spin_unlock_irqrestore(&gi2c->lock, flags);
>> + do {
>> + time_left = wait_for_completion_timeout(&gi2c->done,
>> time_left);
>> + val = readl_relaxed(gi2c->se.base + SE_GENI_M_IRQ_STATUS);
>> + } while (!(val & M_CMD_ABORT_EN) && time_left);
>> +
>> + if (!(val & M_CMD_ABORT_EN) && !time_left)
>
> Why do you need to check !time_left? Just "if (!(val & M_CMD_ABORT_EN))".
>
>
>> + dev_err(gi2c->se.dev, "Timeout abort_m_cmd\n");
>> +}
>> +
>> +static void geni_i2c_rx_fsm_rst(struct geni_i2c_dev *gi2c)
>> +{
>> + u32 val;
>> + unsigned long time_left = RST_TIMEOUT;
>> +
>> + writel_relaxed(1, gi2c->se.base + SE_DMA_RX_FSM_RST);
>> + do {
>> + time_left = wait_for_completion_timeout(&gi2c->done,
>> time_left);
>> + val = readl_relaxed(gi2c->se.base + SE_DMA_RX_IRQ_STAT);
>> + } while (!(val & RX_RESET_DONE) && time_left);
>> +
>> + if (!(val & RX_RESET_DONE) && !time_left)
>
> Similar. Don't need "&& !time_left"
>
>
>> + dev_err(gi2c->se.dev, "Timeout resetting RX_FSM\n");
>> +}
>> +
>> +static void geni_i2c_tx_fsm_rst(struct geni_i2c_dev *gi2c)
>> +{
>> + u32 val;
>> + unsigned long time_left = RST_TIMEOUT;
>> +
>> + writel_relaxed(1, gi2c->se.base + SE_DMA_TX_FSM_RST);
>> + do {
>> + time_left = wait_for_completion_timeout(&gi2c->done,
>> time_left);
>> + val = readl_relaxed(gi2c->se.base + SE_DMA_TX_IRQ_STAT);
>> + } while (!(val & TX_RESET_DONE) && time_left);
>> +
>> + if (!(val & TX_RESET_DONE) && !time_left)
>
> Similar. Don't need "&& !time_left"
>
>
>> +static int geni_i2c_probe(struct platform_device *pdev)
>> +{
>> + struct geni_i2c_dev *gi2c;
>> + struct resource *res;
>> + u32 proto, tx_depth;
>> + int ret;
>> +
>> +
Re: [PATCH v4 4/6] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
On 3/20/2018 9:37 AM, Stephen Boyd wrote:
> Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:49)
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c
>> b/drivers/tty/serial/qcom_geni_serial.c
>> new file mode 100644
>> index 000..1442777
>> --- /dev/null
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -0,0 +1,1158 @@
>> +
>> +#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
>> +static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch)
>> +{
>> + writel_relaxed(ch, uport->membase + SE_GENI_TX_FIFOn);
>
> Does this expect the whole word to have data to write? Or does the FIFO
> output a character followed by three NUL bytes each time it gets
> written? The way that uart_console_write() works is to take each
> character a byte at a time, put it into an int (so extend that byte with
> zero) and then pass it to the putchar function. I would expect that at
> this point the hardware sees the single character and then 3 NULs enter
> the FIFO each time.
>
> For previous MSM uarts I had to handle this oddity by packing the words
> into the fifo four at a time. You may need to do the same here.
The packing configuration 1 * 8 (done using geni_se_config_packing)
ensures that only one byte per FIFO word needs to be transmitted. From
that perspective, we need not have such oddity.
>
>> +}
>> +
>> +static void
>> +__qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
>> +unsigned int count)
>> +{
>> + int i;
>> + u32 bytes_to_send = count;
>> +
>> + for (i = 0; i < count; i++) {
>> + if (s[i] == '\n')
>
> Can you add a comment that uart_console_write() adds a carriage return
> for each newline?
Ok.
>
>> + bytes_to_send++;
>> + }
>> +
>> + writel_relaxed(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
>> + qcom_geni_serial_setup_tx(uport, bytes_to_send);
>> + for (i = 0; i < count; ) {
>> + size_t chars_to_write = 0;
>> + size_t avail = DEF_FIFO_DEPTH_WORDS - DEF_TX_WM;
>> +
>> + /*
>> +* If the WM bit never set, then the Tx state machine is not
>> +* in a valid state, so break, cancel/abort any existing
>> +* command. Unfortunately the current data being written is
>> +* lost.
>> +*/
>> + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
>> + M_TX_FIFO_WATERMARK_EN,
>> true))
>> + break;
>> + chars_to_write = min_t(size_t, (size_t)(count - i), avail /
>> 2);
>
> The _t part of min_t should do the casting already, so we can drop the
> cast on count - i?
Ok.
>
>> + uart_console_write(uport, s + i, chars_to_write,
>> + qcom_geni_serial_wr_char);
>> + writel_relaxed(M_TX_FIFO_WATERMARK_EN, uport->membase +
>> + SE_GENI_M_IRQ_CLEAR);
>> + i += chars_to_write;
>> + }
>> + qcom_geni_serial_poll_tx_done(uport);
>> +}
>> +
>> +static void qcom_geni_serial_console_write(struct console *co, const char
>> *s,
>> + unsigned int count)
>> +{
>> + struct uart_port *uport;
>> + struct qcom_geni_serial_port *port;
>> + bool locked = true;
>> + unsigned long flags;
>> +
>> + WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
>> +
>> + port = get_port_from_line(co->index);
>> + if (IS_ERR(port))
>> + return;
>> +
>> + uport = &port->uport;
>> + if (oops_in_progress)
>> + locked = spin_trylock_irqsave(&uport->lock, flags);
>> + else
>> + spin_lock_irqsave(&uport->lock, flags);
>> +
>> + /* Cancel the current write to log the fault */
>> + if (!locked) {
>> + geni_se_cancel_m_cmd(&port->se);
>> + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
>> + M_CMD_CANCEL_EN, true)) {
>> + geni_se_abort_m_cmd(&port->se);
>> + qcom_geni_serial_poll_bit(uport,
>> SE_GENI_M_IRQ_STATUS,
>> + M_CMD_ABORT_EN,
>> true);
>> + writel_relaxed(M_CMD_ABORT_EN, uport->membase +
>> + SE_GENI_M_IRQ_CLEAR);
>> + }
>> + writel_relaxed(M_CMD_CANCEL_EN, uport->membase +
>> + SE_GENI_M_IRQ_CLEAR);
>> + }
>> +
>> + __qcom_geni_serial_console_write(uport, s, count);
>> + if (locked)
>> + spin_unlock_irqrestore(&uport->lock, flags);
>> +}
>
> Can you also support the OF_EARLYCON_DECLARE method of console writing
Re: [PATCH v4 4/6] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
On 3/20/2018 12:39 PM, Evan Green wrote:
> Hi Karthik,
>
> On Wed, Mar 14, 2018 at 4:59 PM Karthikeyan Ramasubramanian <
> krama...@codeaurora.org> wrote:
>
>> +
>> +static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
>> + int offset, int field, bool set)
>> +{
>> + u32 reg;
>> + struct qcom_geni_serial_port *port;
>> + unsigned int baud;
>> + unsigned int fifo_bits;
>> + unsigned long timeout_us = 2;
>> +
>> + /* Ensure polling is not re-ordered before the prior writes/reads
> */
>> + mb();
>
> These barriers sprinkled around everywhere are new. Did
> you find that you needed them for something? In this case, the
> readl_poll_timeout_atomic should already have a read barrier (nor do you
> probably care about read reordering, right?) Perhaps this should
> only be a mmiowb rather than a full barrier, because you really just want
> to say "make sure all my old writes got out to hardware before spinning"
These barriers have been there from v3. Regarding this barrier - since
readl_poll_timeout_atomic has a read barrier, this one can be converted
to just write barrier.
>
>> +
>> + if (uport->private_data) {
>> + port = to_dev_port(uport, uport);
>> + baud = port->baud;
>> + if (!baud)
>> + baud = 115200;
>> + fifo_bits = port->tx_fifo_depth * port->tx_fifo_width;
>> + /*
>> +* Total polling iterations based on FIFO worth of bytes
> to be
>> +* sent at current baud. Add a little fluff to the wait.
>> +*/
>> + timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
>> + }
>> +
>> + return !readl_poll_timeout_atomic(uport->membase + offset, reg,
>> +(bool)(reg & field) == set, 10, timeout_us);
>> +}
> [...]
>> +
>> +static void qcom_geni_serial_start_tx(struct uart_port *uport)
>> +{
>> + u32 irq_en;
>> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
>> + u32 status;
>> +
>> + if (port->xfer_mode == GENI_SE_FIFO) {
>> + status = readl_relaxed(uport->membase + SE_GENI_STATUS);
>> + if (status & M_GENI_CMD_ACTIVE)
>> + return;
>> +
>> + if (!qcom_geni_serial_tx_empty(uport))
>> + return;
>> +
>> + /*
>> +* Ensure writing to IRQ_EN & watermark registers are not
>> +* re-ordered before checking the status of the Serial
>> +* Engine and TX FIFO
>> +*/
>> + mb();
>
> Here's another one. You should just be able to use a regular readl when
> reading SE_GENI_STATUS and remove this barrier, since readl has an rmb
> which orders your read of M_IRQ_EN below. I don't think you need to worry
> about the writes below going above the read above, since there's logic in
> between that needs the result of the read. Maybe that also saves you in the
> read case, too. Either way, this barrier seems heavy handed.
Write to the watermark register does not have any dependency on the
reads. Hence it can be re-ordered. But writing to that register alone
without enabling the watermark interrupt will not have any impact. From
that perspective, using readl while checking the GENI_STATUS is
sufficient to maintain the required order. I will put a comment
regarding the use of readl so that the reason is not lost.
>
>> +
>> + irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
>> + irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
>> +
>> + writel_relaxed(port->tx_wm, uport->membase +
>> + SE_GENI_TX_WATERMARK_REG);
>> + writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
>> + }
>> +}
>> +
>> +static void qcom_geni_serial_stop_tx(struct uart_port *uport)
>> +{
>> + u32 irq_en;
>> + u32 status;
>> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
>> +
>> + irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
>> + irq_en &= ~M_CMD_DONE_EN;
>> + if (port->xfer_mode == GENI_SE_FIFO) {
>> + irq_en &= ~M_TX_FIFO_WATERMARK_EN;
>> + writel_relaxed(0, uport->membase +
>> +SE_GENI_TX_WATERMARK_REG);
>> + }
>> + port->xmit_size = 0;
>> + writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
>> + status = readl_relaxed(uport->membase + SE_GENI_STATUS);
>> + /* Possible stop tx is called multiple times. */
>> + if (!(status & M_GENI_CMD_ACTIVE))
>> + return;
>> +
>> + /*
>> +* Ensure cancel command write is not re-ordered before checking
>> +* the status of the Primary Sequencer.
>> +*/
>> + mb();
>
> I don't see how what's stated
Re: [PATCH v4 4/6] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
On Tue, Mar 20, 2018 at 4:44 PM Karthik Ramasubramanian <
krama...@codeaurora.org> wrote:
> On 3/20/2018 12:39 PM, Evan Green wrote:
> > Hi Karthik,
> >
> > On Wed, Mar 14, 2018 at 4:59 PM Karthikeyan Ramasubramanian <
> > krama...@codeaurora.org> wrote:
> >
> >> +
> >> +static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
> >> + int offset, int field, bool set)
> >> +{
> >> + u32 reg;
> >> + struct qcom_geni_serial_port *port;
> >> + unsigned int baud;
> >> + unsigned int fifo_bits;
> >> + unsigned long timeout_us = 2;
> >> +
> >> + /* Ensure polling is not re-ordered before the prior
writes/reads
> > */
> >> + mb();
> >
> > These barriers sprinkled around everywhere are new. Did
> > you find that you needed them for something? In this case, the
> > readl_poll_timeout_atomic should already have a read barrier (nor do you
> > probably care about read reordering, right?) Perhaps this should
> > only be a mmiowb rather than a full barrier, because you really just
want
> > to say "make sure all my old writes got out to hardware before spinning"
> These barriers have been there from v3. Regarding this barrier - since
> readl_poll_timeout_atomic has a read barrier, this one can be converted
> to just write barrier.
Thanks. I must have missed them in v3. Is that mmiowb that would go there,
or wmb? I'm unsure.
> >
> >> +
> >> + if (uport->private_data) {
> >> + port = to_dev_port(uport, uport);
> >> + baud = port->baud;
> >> + if (!baud)
> >> + baud = 115200;
> >> + fifo_bits = port->tx_fifo_depth * port->tx_fifo_width;
> >> + /*
> >> +* Total polling iterations based on FIFO worth of
bytes
> > to be
> >> +* sent at current baud. Add a little fluff to the
wait.
> >> +*/
> >> + timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
> >> + }
> >> +
> >> + return !readl_poll_timeout_atomic(uport->membase + offset, reg,
> >> +(bool)(reg & field) == set, 10, timeout_us);
> >> +}
> > [...]
> >> +
> >> +static void qcom_geni_serial_start_tx(struct uart_port *uport)
> >> +{
> >> + u32 irq_en;
> >> + struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
> >> + u32 status;
> >> +
> >> + if (port->xfer_mode == GENI_SE_FIFO) {
> >> + status = readl_relaxed(uport->membase +
SE_GENI_STATUS);
> >> + if (status & M_GENI_CMD_ACTIVE)
> >> + return;
> >> +
> >> + if (!qcom_geni_serial_tx_empty(uport))
> >> + return;
> >> +
> >> + /*
> >> +* Ensure writing to IRQ_EN & watermark registers are
not
> >> +* re-ordered before checking the status of the Serial
> >> +* Engine and TX FIFO
> >> +*/
> >> + mb();
> >
> > Here's another one. You should just be able to use a regular readl when
> > reading SE_GENI_STATUS and remove this barrier, since readl has an rmb
> > which orders your read of M_IRQ_EN below. I don't think you need to
worry
> > about the writes below going above the read above, since there's logic
in
> > between that needs the result of the read. Maybe that also saves you in
the
> > read case, too. Either way, this barrier seems heavy handed.
> Write to the watermark register does not have any dependency on the
> reads. Hence it can be re-ordered. But writing to that register alone
> without enabling the watermark interrupt will not have any impact. From
> that perspective, using readl while checking the GENI_STATUS is
> sufficient to maintain the required order. I will put a comment
> regarding the use of readl so that the reason is not lost.
Yes, I see what you mean, and without the branch I'd agree. My thinking
though was that you have a branch between the read and writes. So to
determine whether or not to even execute the writes, you must have
successfully evaluated the read, since program flow depends on it. I will
admit this is where my barrier knowledge gets fuzzy. Can speculative
execution perform register writes? (ie if that branch was guessed wrong
before the read actually completes, and then the writes happen before the
read? That seems like it couldn't possibly happen, as it would result in
weird spurious speculative writes to registers. Perhaps the mapping bits
prevent this sort of thing...)
If what I've said makes any sort of sense, and you still want to keep the
barriers here and below, then I'll let it go.
> >
> >> +
> >> + irq_en = readl_relaxed(uport->membase +
SE_GENI_M_IRQ_EN);
> >> + irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
> >> +
> >> + writel_relaxed(port->tx_wm, uport->membase +
> >> +
SE_GENI_TX_WATERMARK_REG);
> >> + writel_relaxed(irq_en, upo
Re: [PATCH v4 6/6] arm64: dts: sdm845: Add I2C controller support
Hi,
On Tue, Mar 20, 2018 at 3:16 PM, Sagar Dharia wrote:
> + pinconf {
> + pins = "gpio55", "gpio56";
> + drive-strength = <2>;
> + bias-disable;
> + };
> + };
> +
> + qup-i2c10-sleep {
> + pinconf {
> + pins = "gpio55", "gpio56";
> + bias-pull-up;
Are you sure that you want pullups enabled for sleep here? There are
external pulls on this line (as there are on many i2c busses) so doing
this will double-enable pulls. It probably won't hurt, but I'm
curious if there's some sort of reason here.
>>> 1. We need the lines to remain high to avoid slaves sensing a false
>>> start-condition (this can happen if the SDA goes down before SCL).
>>> 2. Disclaimer: I'm not a HW expert, but we were told that
>>> tri-state/bias-disabled lines can draw more current. I will find out
>>> more about that.
>>
>> Agreed that they need to remain high, but you've got very strong
>> pullups external to the SoC. Those will keep it high. You don't need
>> the internal ones too.
>>
>> As extra evidence that the external pullups _must_ be present on your
>> board: you specify bias-disable in the active state. That can only
>> work if there are external pullups (or if there were some special
>> extra secret internal pullups that were part of geni). i2c is an
>> open-drain bus and thus there must be pullups on the bus in order to
>> communicate.
>>
>
> You are right, I followed up about the pull-up recommendation and that
> was for a GPIO where there was no external pull-up (GPIO was not used
> for I2C). It's safe to assume I2C will always have external pullup.
It is even more safe to say that I2C will always have an external
pullup on the SDM845-MTP. Remember that the pullup config is in the
board device tree file, not the SoC one. So even if someone out there
decides that the internal pull is somehow good enough for their own
board and they don't stuff external ones, then it will be up to them
to turn the pull up on in their own board file.
> We
> will change sleep-config of I2C GPIOs to no-pull.
Even better IMHO: don't specify the bias in the sleep config. I don't
believe it's possible for the sleep config to take effect without the
default config since the default config applies at probe time. ...so
you'll always get the default config applied at probe time and you
don't need to touch the bias at sleep time.
> + i2c10: i2c@a88000 {
Seems like it might be nice to add all the i2c busses into the main
sdm845.dtsi file. Sure, most won't be enabled, but it seems like it
would avoid churn later.
...if you're sure you want to add only one i2c controller, subject of
this patch should indicate that.
>>>
>>> Yes, we typically have a "platform(sdm845 here)-qupv3.dtsi" defining
>>> most of the serial-bus instances (i2c, spi, and uart with
>>> status=disabled) that we include from the common header. The boards
>>> enable instances they need.
>>> Will that be okay?
>>
>> Unless you really feel the need to put these in a separate file I'd
>> just put them straight in sdm845.dtsi. Yeah, it'll get big, but
>> that's OK by me. I _think_ this matches what Bjorn was suggesting on
>> previous device tree patches, but CCing him just in case. I'm
>> personally OK with whatever Bjorn and other folks with more Qualcomm
>> history would like.
>>
>> ...but yeah, I'm asking for them all to be listed with status="disabled".
>>
>
> Sure, we will change the subject of this patch to indicate that we are
> adding 1 controller as of now. Later we will add all I2C controllers to
> dtsi as another patch since that will need pinctrl settings for GPIOs
> used by those instances and the wrappers devices needed by them.
Yeah, it's fine to just change the subject of this patch. It would be
nice to add all the other controllers in sooner rather than later, but
it doesn't have to be today.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 5/9] pwm: tegra: Add PWM based Tachometer driver
PWM Tachometer driver capture the PWM signal which is output of FAN
in general and provide the period of PWM signal which is converted to
RPM by SW.
Add Tegra Tachometer driver which implements the pwm-capture to
measure period.
Signed-off-by: Rajkumar Rampelli
Signed-off-by: Laxman Dewangan
---
V2: Renamed compatible string to "nvidia-tegra186-pwm-tachometer"
Renamed arguments of reset and clk apis to "tachometer" from "tach"
drivers/pwm/Kconfig| 10 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-tegra-tachometer.c | 303 +
3 files changed, 314 insertions(+)
create mode 100644 drivers/pwm/pwm-tegra-tachometer.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 763ee50..29aeeeb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -454,6 +454,16 @@ config PWM_TEGRA
To compile this driver as a module, choose M here: the module
will be called pwm-tegra.
+config PWM_TEGRA_TACHOMETER
+ tristate "NVIDIA Tegra Tachometer PWM driver"
+ depends on ARCH_TEGRA
+ help
+ NVIDIA Tegra Tachometer reads the PWM signal and reports the PWM
+ signal periods. This helps in measuring the fan speed where Fan
+ output for speed is PWM signal.
+
+ This driver support the Tachometer driver in PWM framework.
+
config PWM_TIECAP
tristate "ECAP PWM support"
depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0258a74..14c183e 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PWM_STM32_LP)+= pwm-stm32-lp.o
obj-$(CONFIG_PWM_STMPE)+= pwm-stmpe.o
obj-$(CONFIG_PWM_SUN4I)+= pwm-sun4i.o
obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
+obj-$(CONFIG_PWM_TEGRA_TACHOMETER) += pwm-tegra-tachometer.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o
diff --git a/drivers/pwm/pwm-tegra-tachometer.c
b/drivers/pwm/pwm-tegra-tachometer.c
new file mode 100644
index 000..bcc44ce
--- /dev/null
+++ b/drivers/pwm/pwm-tegra-tachometer.c
@@ -0,0 +1,303 @@
+/*
+ * Tegra Tachometer Pulse-Width-Modulation driver
+ *
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Since oscillator clock (38.4MHz) serves as a clock source for
+ * the tach input controller, 1.0105263MHz (i.e. 38.4/38) has to be
+ * used as a clock value in the RPM calculations
+ */
+#define TACH_COUNTER_CLK 1010526
+
+#define TACH_FAN_TACH0 0x0
+#define TACH_FAN_TACH0_PERIOD_MASK 0x7
+#define TACH_FAN_TACH0_PERIOD_MAX 0x7
+#define TACH_FAN_TACH0_PERIOD_MIN 0x0
+#define TACH_FAN_TACH0_WIN_LENGTH_SHIFT25
+#define TACH_FAN_TACH0_WIN_LENGTH_MASK 0x3
+#define TACH_FAN_TACH0_OVERFLOW_MASK BIT(24)
+
+#define TACH_FAN_TACH1 0x4
+#define TACH_FAN_TACH1_HI_MASK 0x7
+/*
+ * struct pwm_tegra_tach - Tegra tachometer object
+ * @dev: device providing the Tachometer
+ * @pulse_per_rev: Pulses per revolution of a Fan
+ * @capture_window_len: Defines the window of the FAN TACH monitor
+ * @regs: physical base addresses of the controller
+ * @clk: phandle list of tachometer clocks
+ * @rst: phandle to reset the controller
+ * @chip: PWM chip providing this PWM device
+ */
+struct pwm_tegra_tach {
+ struct device *dev;
+ void __iomem*regs;
+ struct clk *clk;
+ struct reset_control*rst;
+ u32 pulse_per_rev;
+ u32 capture_window_len;
+ struct pwm_chip chip;
+};
+
+static struct pwm_tegra_tach *to_tegra_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct pwm_tegra_tach, chip);
+}
+
+static u32 tachometer_readl(struct pwm_tegra_tach *ptt, unsigned long reg)
+{
+ return readl(ptt->regs + reg);
+}
+
+static inline void tachometer_writel(struct pwm_tegra_tach *ptt, u32 val,
+unsigned long reg)
+{
+ writel(val, ptt->regs + reg);
+}
+
+static int pwm_tegra_tach_set_wlen(struct pwm_tegra_tach *ptt,
+
[PATCH V2 7/9] hwmon: pwm-fan: add sysfs node to read rpm of fan
Add fan device attribute fan1_input in pwm-fan driver
to read speed of fan in rotations per minute.
Signed-off-by: Rajkumar Rampelli
---
V2: Removed generic-pwm-tachometer driver and using pwm-fan driver as per
suggestions
to read fan speed.
Added fan device attribute to report speed of fan in rpms through hwmon
sysfs.
drivers/hwmon/pwm-fan.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 70cc0d1..8dda209 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -98,11 +98,34 @@ static ssize_t show_pwm(struct device *dev,
return sprintf(buf, "%u\n", ctx->pwm_value);
}
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_fan_ctx *ptt = dev_get_drvdata(dev);
+ struct pwm_device *pwm = ptt->pwm;
+ struct pwm_capture result;
+ unsigned int rpm = 0;
+ int ret;
+
+ ret = pwm_capture(pwm, &result, 0);
+ if (ret < 0) {
+ pr_err("Failed to capture PWM: %d\n", ret);
+ return ret;
+ }
+
+ if (result.period)
+ rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+ result.period);
+
+ return sprintf(buf, "%u\n", rpm);
+}
static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444, show_rpm, NULL, 0);
static struct attribute *pwm_fan_attrs[] = {
&sensor_dev_attr_pwm1.dev_attr.attr,
+ &sensor_dev_attr_fan1_input.dev_attr.attr,
NULL,
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 8/9] arm64: defconfig: enable Nvidia Tegra Tachometer as a module
Tegra Tachometer driver implements PWM capture to measure period. Enable this driver as a module in the ARM64 defconfig. Signed-off-by: Rajkumar Rampelli --- V2: No changes in this patch arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 634b373..8b2bda7 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -550,6 +550,7 @@ CONFIG_PWM_MESON=m CONFIG_PWM_ROCKCHIP=y CONFIG_PWM_SAMSUNG=y CONFIG_PWM_TEGRA=m +CONFIG_PWM_TEGRA_TACHOMETER=m CONFIG_PHY_RCAR_GEN3_USB2=y CONFIG_PHY_HI6220_USB=y CONFIG_PHY_QCOM_USB_HS=y -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 9/9] arm64: defconfig: enable pwm-fan as a loadable module
Enable pwm-fan driver to make use of a PWM interface to read speed of a fan in rotations per minute. Signed-off-by: Rajkumar Rampelli --- V2: Added pwm-fan driver support as a loadable module. Removed generic-pwm-tachometer driver support which was added as part of v1 arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 8b2bda7..50aa3bce 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -320,6 +320,7 @@ CONFIG_SYSCON_REBOOT_MODE=y CONFIG_BATTERY_BQ27XXX=y CONFIG_SENSORS_ARM_SCPI=y CONFIG_SENSORS_LM90=m +CONFIG_SENSORS_PWM_FAN=m CONFIG_SENSORS_INA2XX=m CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y CONFIG_CPU_THERMAL=y -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 2/9] arm64: tegra: Add PWM controller on Tegra186 soc
The NVIDIA Tegra186 SoC has a PWM controller which is
used in FAN control use case.
Signed-off-by: Rajkumar Rampelli
---
V2: no changes in this patch
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index b762227..731cd01 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1031,4 +1031,15 @@
(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
interrupt-parent = <&gic>;
};
+
+ pwm@c34 {
+ compatible = "nvidia,tegra186-pwm";
+ reg = <0x0 0xc34 0x0 0x1>;
+ clocks = <&bpmp TEGRA186_CLK_PWM4>;
+ clock-names = "pwm";
+ #pwm-cells = <2>;
+ resets = <&bpmp TEGRA186_RESET_PWM4>;
+ reset-names = "pwm";
+ status = "disabled";
+ };
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 1/9] pwm: core: Add support for PWM HW driver with pwm capture only
Add support for pwm HW driver which has only capture functionality.
This helps to implement the PWM based Tachometer driver which reads
the PWM output signals from electronic fans.
PWM Tachometer captures the period and duty cycle of the PWM signal
Add conditional checks for callabacks enable(), disable(), config()
to check if they are supported by the client driver or not. Skip these
callbacks if they are not supported.
Signed-off-by: Rajkumar Rampelli
---
V2: Added if conditional checks for pwm callbacks since drivers may
implements only pwm capture functionality.
drivers/pwm/core.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1581f6a..f70fe68 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -246,6 +246,10 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
if (ops->apply)
return true;
+ /* driver supports capture operation */
+ if (ops->capture)
+ return true;
+
return false;
}
@@ -495,7 +499,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct
pwm_state *state)
* ->apply().
*/
if (pwm->state.enabled) {
- pwm->chip->ops->disable(pwm->chip, pwm);
+ if (pwm->chip->ops->disable)
+ pwm->chip->ops->disable(pwm->chip, pwm);
pwm->state.enabled = false;
}
@@ -509,22 +514,26 @@ int pwm_apply_state(struct pwm_device *pwm, struct
pwm_state *state)
if (state->period != pwm->state.period ||
state->duty_cycle != pwm->state.duty_cycle) {
- err = pwm->chip->ops->config(pwm->chip, pwm,
+ if (pwm->chip->ops->config) {
+ err = pwm->chip->ops->config(pwm->chip, pwm,
state->duty_cycle,
state->period);
- if (err)
- return err;
+ if (err)
+ return err;
+ }
pwm->state.duty_cycle = state->duty_cycle;
pwm->state.period = state->period;
}
if (state->enabled != pwm->state.enabled) {
- if (state->enabled) {
+ if (state->enabled && pwm->chip->ops->enable) {
err = pwm->chip->ops->enable(pwm->chip, pwm);
if (err)
return err;
- } else {
+ }
+
+ if (!state->enabled && pwm->chip->ops->disable) {
pwm->chip->ops->disable(pwm->chip, pwm);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 4/9] arm64: tegra: Add Tachometer Controller on Tegra186
The NVIDIA Tegra186 SoC has a Tachometer Controller that analyzes the
PWM signal of a Fan and reports the period value through pwm interface.
Signed-off-by: Rajkumar Rampelli
---
V2: Renamed clock-names/reset-names dt properties values to "tachometer"
Renamed compatible property value to "nvidia-tegra186-pwm-tachometer"
arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts | 5 +
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++
2 files changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts
b/arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts
index bd5305a..13c3e59 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts
@@ -172,4 +172,9 @@
vin-supply = <&vdd_5v0_sys>;
};
};
+
+ tachometer@39c {
+ nvidia,pulse-per-rev = <2>;
+ nvidia,capture-window-len = <2>;
+ };
};
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 731cd01..19e1afc 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1042,4 +1042,15 @@
reset-names = "pwm";
status = "disabled";
};
+
+ tegra_tachometer: tachometer@39c {
+ compatible = "nvidia,tegra186-pwm-tachometer";
+ reg = <0x0 0x039c 0x0 0x10>;
+ #pwm-cells = <2>;
+ clocks = <&bpmp TEGRA186_CLK_TACH>;
+ clock-names = "tachometer";
+ resets = <&bpmp TEGRA186_RESET_TACH>;
+ reset-names = "tachometer";
+ status = "disabled";
+ };
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings
Supply Device tree binding documentation for the NVIDIA
Tegra186 SoC's Tachometer Controller
Signed-off-by: Rajkumar Rampelli
---
V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
Renamed dt property values of clock-names and reset-names to "tachometer"
from "tach"
.../bindings/pwm/pwm-tegra-tachometer.txt | 31 ++
1 file changed, 31 insertions(+)
create mode 100644
Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
diff --git a/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
new file mode 100644
index 000..4a7ead4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
@@ -0,0 +1,31 @@
+Bindings for a PWM based Tachometer driver
+
+Required properties:
+- compatible: Must be "nvidia,tegra186-pwm-tachometer"
+- reg: physical base addresses of the controller and length of
+ memory mapped region.
+- #pwm-cells: should be 2. See pwm.txt in this directory for a
+ description of the cells format.
+- clocks: phandle list of tachometer clocks
+- clock-names: should be "tachometer". See clock-bindings.txt in documentations
+- resets: phandle to the reset controller for the Tachometer IP
+- reset-names: should be "tachometer". See reset.txt in documentations
+- nvidia,pulse-per-rev: Integer, pulses per revolution of a Fan. This value
+ obtained from Fan specification document.
+- nvidia,capture-window-len: Integer, window of the Fan Tach monitor, it
indicates
+ that how many period of the input fan tach signal will the FAN TACH logic
+ monitor. Valid values are 1, 2, 4 and 8 only.
+
+Example:
+ tegra_tachometer: tachometer@39c {
+ compatible = "nvidia,tegra186-pwm-tachometer";
+ reg = <0x0 0x039c 0x0 0x10>;
+ #pwm-cells = <2>;
+ clocks = <&tegra_car TEGRA186_CLK_TACH>;
+ clock-names = "tachometer";
+ resets = <&tegra_car TEGRA186_RESET_TACH>;
+ reset-names = "tachometer";
+ nvidia,pulse-per-rev = <2>;
+ nvidia,capture-window-len = <2>;
+ status = "disabled";
+ };
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 0/9] Implementation of Tegra Tachometer driver
The following patches adds support for PWM based Tegra Tachometer driver which implements PWM capture interface to analyze the PWM signal of a electronic fan and reports it in periods and duty cycles. Added fan device attribute fan1_input in pwm-fan driver to monitor the speed of fan in rotations per minute using PWM interface. RPM of Fan will be exposed to user interface through HWMON sysfs interface avialable at location: /sys/class/hwmon/hwmon0/fan1_input Steps to validate Tachometer on Tegra186 SoC: A. push modules pwm-tegra.ko, pwm-tegra-tachometer.ko and pwm-fan.ko to linux device using scp command. scp build/tegra186/drivers/pwm/pwm-tegra.ko ubuntu@10.19.65.176:/tmp/ scp build/tegra186/drivers/pwm/pwm-tegra-tachometer.ko ubuntu@10.19.65.176:/tmp/ scp build/tegra186/drivers/hwmon/pwm-fan.ko ubuntu@10.19.65.176:/tmp/ B. On Linux device console, insert these modules using insmod command. insmod /tmp/pwm-tegra.ko insmod /tmp/pwm-tegra-tachometer.ko insmod /tmp/pwm-fan.ko C. Read RPM value at below sysfs node cat /sys/calss/hwmon/hwmon0/fan1_input D. Change the FAN speed using PWM sysfs interface. Follow below steps for the same: a. cd /sys/class/pwm/pwmchip0 b. ls -la (make sure pwm controller is c34.pwm) Output should be: device -> ../../../c34.pwm c. echo 0 > export d. cd pwmchip0:0 e. echo 8000 > period f. echo 1 > enable g. echo 8000 > duty_cycle # change duty_cycles from 0 to 8000 for FAN speed variation h. cat /sys/calss/hwmon/hwmon0/fan1_input i. echo 4000 > duty_cycle h. cat /sys/calss/hwmon/hwmon0/fan1_input i. echo 2000 > duty_cycle h. cat /sys/calss/hwmon/hwmon0/fan1_input i. echo 0 > duty_cycle h. cat /sys/calss/hwmon/hwmon0/fan1_input Rajkumar Rampelli (9): pwm: core: Add support for PWM HW driver with pwm capture only arm64: tegra: Add PWM controller on Tegra186 soc dt-bindings: Tegra186 tachometer device tree bindings arm64: tegra: Add Tachometer Controller on Tegra186 pwm: tegra: Add PWM based Tachometer driver arm64: tegra: Add pwm based fan support on Tegra186 hwmon: pwm-fan: add sysfs node to read rpm of fan arm64: defconfig: enable Nvidia Tegra Tachometer as a module arm64: defconfig: enable pwm-fan as a loadable module .../bindings/pwm/pwm-tegra-tachometer.txt | 31 +++ arch/arm64/boot/dts/nvidia/tegra186-p2771-.dts | 5 + arch/arm64/boot/dts/nvidia/tegra186.dtsi | 28 ++ arch/arm64/configs/defconfig | 2 + drivers/hwmon/pwm-fan.c| 23 ++ drivers/pwm/Kconfig| 10 + drivers/pwm/Makefile | 1 + drivers/pwm/core.c | 21 +- drivers/pwm/pwm-tegra-tachometer.c | 303 + 9 files changed, 418 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt create mode 100644 drivers/pwm/pwm-tegra-tachometer.c -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 6/9] arm64: tegra: Add pwm based fan support on Tegra186
Add pwm fan driver support on Tegra186 SoC.
Signed-off-by: Rajkumar Rampelli
---
V2: Removed generic-pwm-tachometer driver dt node and using pwm-fan driver
for reading fan speed.
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 19e1afc..27ae73e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1053,4 +1053,10 @@
reset-names = "tachometer";
status = "disabled";
};
+
+ pwm_fan {
+ compatible = "pwm-fan";
+ pwms = <&tegra_tachometer 0 100>;
+ status = "disabled";
+ };
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] x86/smpboot: Add the missing description of possible_cpus
Hi Peter,
At 03/20/2018 08:37 PM, Peter Zijlstra wrote:
On Tue, Mar 20, 2018 at 07:04:28PM +0800, Dou Liyang wrote:
+ possible_cpus= [s390,x86_64] Use this to set hotpluggable cpus.
+ This option sets possible_cpus bits in cpu_possible_map.
+ Thus keeping the numbers of bits set constant even if
+ the machine gets rebooted.
That description, esp. the last sentence, doesn't make any kind of sense
to me. What?
Ah, sure enough, I can't be lazy. :-) I stole that from the commit
3b11ce7f542e ("x86: use possible_cpus=NUM to extend the possible cpus
allowed")
How about:
possible_cpus= [s390,x86_64] Set the number of possible CPUs which
are determined by the ACPI tables MADT or mptables by
default. possible_cpus=n : n >= 1 enforces the possible
number to be 'n'.
While nr_cpus is also be set: nr_cpus=m, choice the
minimum one for the number of possible CPUs.
Thank,
dou
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] x86/smpboot: Make the check code more clear in prefill_possible_map()
Hi Peter, At 03/20/2018 08:39 PM, Peter Zijlstra wrote: On Tue, Mar 20, 2018 at 07:04:30PM +0800, Dou Liyang wrote: case 1: no | no | no | --> min (setup_possible_cpus, nr_cpu_ids, setup_max_cpus) case 2: no | no | yes| --> min (setup_possible_cpus, nr_cpu_ids) case 3: no | yes | no | --> 1 case 4: no | yes | yes| --> 1 case 5: yes | no | no | --> min (num_processors, nr_cpu_ids, setup_max_cpus) case 6: yes | no | yes| --> min (num_processors + disabled_cpus, nr_cpu_ids) case 7: yes | yes | no | --> 1 case 8: yes | yes | yes| --> 1 The case number is off by one ;-) I got it! ;-） Thanks dou -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] hwmon: pwm-fan: add sysfs node to read rpm of fan
On 03/20/2018 09:40 PM, Rajkumar Rampelli wrote:
Add fan device attribute fan1_input in pwm-fan driver
to read speed of fan in rotations per minute.
Signed-off-by: Rajkumar Rampelli
---
V2: Removed generic-pwm-tachometer driver and using pwm-fan driver as per
suggestions
to read fan speed.
Added fan device attribute to report speed of fan in rpms through hwmon
sysfs.
drivers/hwmon/pwm-fan.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 70cc0d1..8dda209 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -98,11 +98,34 @@ static ssize_t show_pwm(struct device *dev,
return sprintf(buf, "%u\n", ctx->pwm_value);
}
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_fan_ctx *ptt = dev_get_drvdata(dev);
+ struct pwm_device *pwm = ptt->pwm;
+ struct pwm_capture result;
+ unsigned int rpm = 0;
+ int ret;
+
+ ret = pwm_capture(pwm, &result, 0);
+ if (ret < 0) {
+ pr_err("Failed to capture PWM: %d\n", ret);
+ return ret;
+ }
+
+ if (result.period)
+ rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+ result.period);
+
+ return sprintf(buf, "%u\n", rpm);
+}
static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444, show_rpm, NULL, 0);
static struct attribute *pwm_fan_attrs[] = {
&sensor_dev_attr_pwm1.dev_attr.attr,
+ &sensor_dev_attr_fan1_input.dev_attr.attr,
This doesn't make sense. The same pwm can not both control the fan speed
and report it.
Guenter
NULL,
};
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
